[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #4032: syslog: fix crash when print localtime by syslog

[GitBox <gi...@apache.org>]

xiaoxiang781216 commented on pull request #4032:
URL: https://github.com/apache/incubator-nuttx/pull/4032#issuecomment-872962876


   > @Donny9 What was the problem with this? I tried it, but I didn't have any kind of "crash", it worked OK for me.
   > 
   > I am following the changes, but I don't understand the functional changes this PR brings. It seems to me only "styling" changes are made. Can you please elaborate on the issue and the solution?
   > 
   
   The key change is move localtime_r/gmtime_r into if (OSINIT_HW_READY()).
   
   > Also, to my understanding this PR introduces a bug.
   > If `OSINIT_HW_READY()` returns false, then the variable `tm` will be used uninitialized in line 132.
   > The same applies for variable `ts` in line 140.
   > 
   
   ts and tm is initialized to zero at line 74-76 and 79-81.
   
   > Also, why ingore the return code of the various calls in `clock_gettime()`, `clock_gettime()`, `clock_systime_timespec()`?
   > Isn't it better to check for the error, and provide sane values in `ts` and `tm`?
   
   The sane value(zero) set at the definition.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org