El 11/11/2023 a las 6:57, Brian Inglis escribió:
> On 2023-11-10 10:44, Pedro Luis Castedo Cepeda wrote:
>> El 10/11/2023 a las 11:16, Corinna Vinschen escribió:
>>> On Nov  9 23:17, Brian Inglis wrote:
>>>> On 2023-11-09 12:04, Pedro Luis Castedo Cepeda wrote:
>>>>> - Prevent strftime to parsing format string beyond its end when
>>>>>     it finish with "%E" or "%O".
>>>>> ---
>>>>>    newlib/libc/time/strftime.c | 2 ++
>>>>>    1 file changed, 2 insertions(+)
>>>>>
>>>>> diff --git a/newlib/libc/time/strftime.c b/newlib/libc/time/strftime.c
>>>>> index 56f227c5f..c4e9e45a9 100644
>>>>> --- a/newlib/libc/time/strftime.c
>>>>> +++ b/newlib/libc/time/strftime.c
>>>>> @@ -754,6 +754,8 @@ __strftime (CHAR *s, size_t maxsize, const CHAR 
>>>>> *format,
>>>>>          switch (*format)
>>>>>        {
>>>>> +    case CQ('\0'):
>>>>> +      break;
>>>>>        case CQ('a'):
>>>>>          _ctloc (wday[tim_p->tm_wday]);
>>>>>          for (i = 0; i < ctloclen; i++)
>>>>
>>>> These cases appear to already be taken care of by setting and using
>>>> (depending on the config parameters) the "alt" variable for those 
>>>> modifiers,
>>>> and the default: return 0; for the format *character* (possibly 
>>>> wide) not
>>>> matching following any modifiers.
>>>>
>>>> Patches to newlib should go to the newlib mailing list at sourceware 
>>>> dot org.
>>>
>>> Also, a simple reproducer would be nice.
> 
>> My first contribution. Sorry about posting to wrong mail list and, at 
>> best, minimalistic patch motivation reasoning. First time with git 
>> send-mail, too.
>>
>> I came across this newlib "feature" trying to update GLib port to 
>> 2.78.1. When trying to find out why test_strftime (glib/test/date.c)
>> was failing I discovered that one of the test format strings, "%E" was 
>> triggering a loop in g_date_strftime (glib/gdate.c) requiring more and 
>> more memory till it was stopped by a fortunate maximum size check in 
>> function.
>>
>> The problem is that __strftime  (newlib/libc/time/strftime.c) doesn't 
>> check for '\0' after a terminal "%E" and it continues parsing the 
>> format string. Finally (not sure if intentionally), this triggers a 
>> direct return 0 from __strftime instead breaking the loop, preventing 
>> it from add '\0' to the end of returned string. Same for "%O", I think 
>> (not tested).
>>
>> It seems that this trailing '\0' allows to differentiate returning an 
>> empty string from needing more space (at least, in Glib).
>>
>> So, is it a newlib bug? Not really, I think this format string is 
>> bad-formed (%E should modify something, shouldn't it?) So undefined 
>> behaviour is OK. I could patch-out these format strings from the port.
>>
>> But... from Glib tests, it seems that, at least:
>>
>> - If G_OS_WIN32, terminal "%E" & "%O" are silently discarded.
>> - If __FreeBSD__ || __OpenBSD__ || __APPLE__ they are transformed to E 
>> & O, respectively.
>> - And if #else the same thing is expected.
>>
>> So it seems that returning 0-terminated string is a common practice 
>> and I also think that this is more deterministic and, potentially, 
>> safer. That's why I sent the patch. It tries to be the shortest 
>> addition to check for end of string after %E & %O modifiers and takes 
>> G_OS_WIN32 approach (only cause it's the simplest).
> 
> Not seeing any issue with any format - see attached source and log 
> output, built under Cygwin.
> [Derived from a bash script using printf %(...)T to do the same thing.]
> 

OK. It's not a newlib problem but a GLib one as it is relaying on common 
but non-standard strftime implementation details.

I attach a short program more focused in g_date_strftime implementation 
so it can be evaluated if it worths addressing this corner case.

Thanks.