• State Resolved
  • Locked Locked
  • Replies 6 replies
  • Answers 1 answer
  • Subscribers 80 subscribers
  • Views 672 views
  • Users 0 members are here
Support feedback
Options
Options
Related

This thread has been locked.

If you have a related question, please click the "Ask a related question" button in the top right corner. The newly created question will be automatically linked to this question.

Compiler/MSP430G2553: Updating an int struct member via pointer reference fails to persist

Michael Cress
Expert 1455 points
Part Number: MSP430G2553


Tool/software: TI C/C++ Compiler

Greetings:

In the attached code, the struct member "delay_counter" fails to persist it's updated value within the BlinkLED function. I have not been able to figure out why this is. Is this a compiler bug or architecture limitation?

Thanks,

Michael Cress

TimerDevelopment.zip

  • 0
    Guru 100785 points
    I don't see where you allocate TimerMgr.TasksArr, so I suspect it's NULL when you go to use it.
    ---------------
    > InsertTimerTask( data->timermgr, BlinkLED, data->timermgr, temp /* data->delay_counter */ );
    This is passing a (TimerMgr *), not a (BlinkLEDData *), to the next call of BlinkLED. I suspect that will cause trouble.
    ---------------
    I'm not sure I know what a failure to persist is, but it seems probable that one of these items is causing it.

    I see no evidence of a compiler bug or architecture limitation.
  • 0 in reply to Bruce McKenney
    Expert 1455 points

    Bruce:

    Thanks for your reply. The array is initialized within InsertTimerTask; when the first item is inserted it will check the array capacity. Seeing it is 0, it will then malloc and memset the array to grow it. Your comment did spur my to add a free() for the array within FreeTimerMgr() though.

    I also updated line 27 of BlinkLED() ( the call to InsertTimerTask ), to pass in Data and not data->timermgr. This is more correct and is a good catch, but makes no difference to the program execution as data->timermgr is the first element of the BlinkLEDData  struct. So, when it gets cast, the cast is pointing to the correct position in memory. This was purely chance and would have resulted in a bug if i had inserted something before it at a later time. Good catch.

    As for persistence, I am referring to the setting of a value and expecting to see it remain the same during the next iteration of BlinkLED(). Even with the corrections I just mentioned, I still don't see the value updating. To compound my frustration, if I compile with an older version of the TI compiler and with GCC, I get different program behaviors despite optimizations being turned off.

    This is really weird behavior...

    I am attaching the new project with the referenced modifications.

    Thanks,

    Michael Cress

    4300.TimerDevelopment.zip

  • 0 in reply to Michael Cress
    Expert 1455 points
    As an aside, I moved the data variable out of main() and made it globally static with no change in program behavior.
  • 0 in reply to Michael Cress
    Expert 1455 points
    Update:

    WITH GCC, the program works correctly if and only if data is globally static. If I move it back into main() then it doesn't work again.

    WITH TI compilers, the issue remains.
  • 0 in reply to Michael Cress
    Guru 100785 points
    > data->timermgr is the first element of the BlinkLEDData struct
    The start of BlinkLEDData is a (TimerMgr *), not a (TimerMgr). It wasn't even accidentally correct.

    > Mgr->TasksArr = realloc( Mgr->TasksArr, Mgr->NumberOfTasks );
    1) This specifies an incorrect byte count, truncating the TasksArr to one word (not one struct).
    2) TaskArrCapacity isn't adjusted, so the next insertion will cause an array overrun.
    Either of these will sooner or later result in a mangled heap.

    I suggest:

    > Mgr->TasksArr = realloc( Mgr->TasksArr, sizeof(struct TimerTask) *Mgr->NumberOfTasks );
    > if (Mgr->TasksArr == 0) { while (1);}
    > Mgr->TaskArrCapacity -= numexecuted;

    Unsolicited: Your quantum appears to be around 100usec, but as near as I can guess the processing takes longer than that, so your clock will run slow.
  • 0 in reply to Bruce McKenney
    Expert 1455 points
    Bruce:

    Looks like that works! Excellent...thank you very much for your help spotting those errors.

    As for the quantum, I assumed it would run slow but I have no way of measuring it so I am accepting the error. It is just for a lighting circuit, so I don't think it'll be that big of a deal.

    Thanks again!

    Michael

**Attention** This is a public forum