RIOT-OS / RIOT

RIOT - The friendly OS for IoT
https://riot-os.org
GNU Lesser General Public License v2.1
4.91k stars 1.98k forks source link

z1: malloc doesn't work when running z1 mote on Cooja #6150

Closed Jiffkuo closed 6 years ago

Jiffkuo commented 7 years ago

I found an issue about malloc, if you use malloc in C code and compile the code within z1 board then running it on Cooja simulator. No error report after the compilation. However, malloc always returns NULL.

After investigation, I think we can modify msp430-main.c:141 as below,

 if (incr > (unsigned int)(stack_pointer - cur_break)) {
  ...
 }

Could you help to review the modification? Does it make sense? Thank you

Tzu-Chi

OlegHahm commented 7 years ago

First of all: are you sure you want to use malloc() on embedded device - particular a really constrained one such as the MSP430?

Second: I'm not really sure I understand your proposed solution. Have you verified that it fixes your problem? Can you elaborate why casting the result to unsigned int should make a difference?

Jiffkuo commented 7 years ago

Hi,

Thanks for your prompt reply.

  1. Yes, I agree with you. It's better to use static instead of dynamic memory allocation on embedded device. Right now, I am doing an experiment about memory performance and management, so I need to use malloc to allocate some space dynamic. Of course I do monitor the size of malloc in my code.
  2. Yes, I did verify it. The reason is I found if the value of cur_break is greater than the value stack_pointer, which means "stack_pointer - cur_break" generates negative result (e.g. 0x fdfe), it makes 'if (size > (stack_pointer - cur_break))' statement always false. I think it's good to ensure no overlapping on stack_pointer, but we should calculate memory address with unsigned or absolute value. So that's why I cast 'unsigned int' with the statement. It's my 2 cents.

Thanks.

Best regards, Tzu-Chi

2016-11-21 6:31 GMT-08:00 Oleg Hahm notifications@github.com:

First of all: are you sure you want to use malloc() on embedded device - particular a really constrained one such as the MSP430?

Second: I'm not really sure I understand your proposed solution. Have you verified that it fixes your problem? Can you elaborate why casting the result to unsigned int should make a difference?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/RIOT-OS/RIOT/issues/6150#issuecomment-261953113, or mute the thread https://github.com/notifications/unsubscribe-auth/AWhB1eMzR4ykskz_NXlCvGo0edNnnwLJks5rAas3gaJpZM4K3eC2 .

Jiffkuo commented 7 years ago

By the way, I realized that 'unsigned int' may be not good solution for all cases (e.g. address starts with 0xfxxx_xxxx). The point is that we should get the absolute offset value during the address calculation.

2016-11-21 10:54 GMT-08:00 Jiff Kuo jiffkuo@gmail.com:

Hi,

Thanks for your prompt reply.

  1. Yes, I agree with you. It's better to use static instead of dynamic memory allocation on embedded device. Right now, I am doing an experiment about memory performance and management, so I need to use malloc to allocate some space dynamic. Of course I do monitor the size of malloc in my code.
  2. Yes, I did verify it. The reason is I found if the value of cur_break is greater than the value stack_pointer, which means "stack_pointer - cur_break" generates negative result (e.g. 0x fdfe), it makes 'if (size > (stack_pointer - cur_break))' statement always false. I think it's good to ensure no overlapping on stack_pointer, but we should calculate memory address with unsigned or absolute value. So that's why I cast 'unsigned int' with the statement. It's my 2 cents.

Thanks.

Best regards, Tzu-Chi

2016-11-21 6:31 GMT-08:00 Oleg Hahm notifications@github.com:

First of all: are you sure you want to use malloc() on embedded device - particular a really constrained one such as the MSP430?

Second: I'm not really sure I understand your proposed solution. Have you verified that it fixes your problem? Can you elaborate why casting the result to unsigned int should make a difference?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/RIOT-OS/RIOT/issues/6150#issuecomment-261953113, or mute the thread https://github.com/notifications/unsubscribe-auth/AWhB1eMzR4ykskz_NXlCvGo0edNnnwLJks5rAas3gaJpZM4K3eC2 .

PeterKietzmann commented 6 years ago

@Jiffkuo I didn't follow your description above in detail but however, cooja was never (really) part of RIOT ( #1921 wasn't merged). Also, there was no conversation since one year. You think we can close this one?

Jiffkuo commented 6 years ago

Yes, please close it. Thanks

From Jiff Kuo iPhone

Peter Kietzmann notifications@github.com 於 2017年11月27日 09:25 寫道:

@Jiffkuo I didn't follow your description above in detail but however, cooja was never (really) part of RIOT ( #1921 wasn't merged). Also, there was no conversation since one year. You think we can close this one?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

jabberimaher commented 4 years ago

hi, please how integrate cooja in riot os? thank you