> On 9 May 2017, at 09:35, walter harms <[email protected]> wrote:
> 
> Since this is a musl (only ?) feature i would suggest
> to make that selectable.
> 
> re,
> wh

I wouldn’t peg this as a musl-only feature — in fact, their argument
seems to be that this is the only way a POSIX-conformant syslog() can
behave, as it would otherwise modify global state (through settz()
through localtime_r()) that it would not be permitted to modify.

Given the very modest code increase I do not particularly see the point
of it being configurable at build time, but if others concur I’ll change it.

>> 
>> @@ -815,17 +820,23 @@ static void timestamp_and_log(int pri, char *msg, int 
>> len)
>> {
>>      char *timestamp;
>>      time_t now;
>> +    struct tm nowtm = { .tm_isdst = 0 };
> why ? is there a bug ?

strptime() may not set the tm_isdst field, as a result of which it would have
an undefined value if the struct is not explicitly initialised, which messes
with the calculation in mktime().

>> 
>>      /* Jan 18 00:11:22 msg... */
>>      /* 01234567890123456 */
>>      if (len < 16 || msg[3] != ' ' || msg[6] != ' '
>>       || msg[9] != ':' || msg[12] != ':' || msg[15] != ' '
>>      ) {
>> -            time(&now);
>> +            now = time(NULL);
> 
> is the same

I just figured it’s nicer if the code paths look similar.

>>              timestamp = ctime(&now) + 4; /* skip day of week */
>>      } else {
>> -            now = 0;
>> -            timestamp = msg;
>> +            if (G.adjustTimezone && strptime(msg, "%b %e %T", &nowtm)) {
>> +                    now = mktime(&nowtm) - timezone;
>> +                    timestamp = ctime(&now) + 4; /* skip day of week */
>> +            } else {
>> +                    now = 0;
>> +                    timestamp = msg;
>> +            }
> 
> It is more easy to disable the feature if you do
>               now = 0;
>               timestamp = msg;
>               if (G.adjustTimezone && strptime(msg, "%b %e %T", &nowtm)) {
>                       now = mktime(&nowtm) - timezone;
>                       timestamp = ctime(&now) + 4; /* skip day of week */
>               }

Right, I’ll change that as well if the consensus is that it being selectable
is better.

Thanks for the feedback!

- Shiz

Attachment: signature.asc
Description: Message signed with OpenPGP

_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox

Reply via email to