Ping.

- Shiz

> On 9 May 2017, at 15:13, Shiz <[email protected]> wrote:
> 
> 
>> 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