syslog: fix crash when print localtime by syslog

[Donny9]

Summary

syslog: fix crash when print localtime by syslog

test need to enable config:
CONFIG_LIBC_LOCATIME=y
CONFIG_SYSLOG_TIMESTAMP_FORMATTED=y
CONFIG_SYSLOG_TIMESTAMP_LOCALTIME=y
CONFIG_SYSLOG_TIMESTAMP_REALTIME=y

Change-Id: I322830e0818237a7eb65a158a6a0dea8f9da9b6c
Signed-off-by: Jiuzhu Dong dongjiuzhu1@xiaomi.com

Impact

output local time in syslog.

Testing

daily test.

[fjpanag]

@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?

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.

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?

[Donny9]

@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?

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.

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?

Hello @fjpanag , you need to enable follow config, then use syslog to output log with localtime in origin code base.
CONFIG_LIBC_LOCATIME=y
CONFIG_SYSLOG_TIMESTAMP_FORMATTED=y
CONFIG_SYSLOG_TIMESTAMP_LOCALTIME=y
CONFIG_SYSLOG_TIMESTAMP_REALTIME=y

The root cause of cash is that the localtime_r can't be called before OSINIT_HW_READY, because localtime_r will call sem_wait, but semaphore subsystem is uninitialized.

[xiaoxiang781216]

@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.

[fjpanag]

@Donny9 I now understand the issue, thank you!
BTW, as I see, there is no CONFIG_LIBC_LOCATIME defined anywhere....

@xiaoxiang781216 You are correct.
I followed the code with my debugger, and saw the variables containing garbage. But it was my mistake, they are in fact initialized before being used, as they should.

[xiaoxiang781216]

Yes, CONFIG_LIBC_LOCATIME is enabled in our product to convert UTC to local time correclty, which is seldom selected in the demo project. That's why the problem is found until now.