This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] vfprintf: Remove alloca from string formatting with conversion



On 28/06/2017 16:28, Florian Weimer wrote:
> On 06/28/2017 08:46 PM, Adhemerval Zanella wrote:
>>
>>
>> On 28/06/2017 15:13, Florian Weimer wrote:
>>> On 06/27/2017 10:32 PM, Adhemerval Zanella wrote:
>>>
>>>> Wouldn't be better to add a wrapper on malloc/malloc-internal for overflow
>>>> check? Something like
>>>>
>>>> static inline bool
>>>> check_add_overflow_int_t (int left, int right, int *result)
>>>> {
>>>> #if __GNUC__ >= 5
>>>>   return __builtin_add_overflow (left, right, result);
>>>> #else
>>>>   if (INT_MAX - left <= right)
>>>>     return true;
>>>>   *result = left + right;
>>>>   return false;
>>>> #endif
>>>> }
>>>>
>>>> As we already do for multiplication and for unsigned (from my char_array)?
>>>
>>> I'd prefer to wait until we can require GCC 5 as the baseline compiler
>>> and then use the built-ins directly.  Less cognitive load for the
>>> maintainers.
>>
>> This would take at least one or two more releases and I do not see why
>> add and follow an idea already in place for malloc.
> 
> GCC 4.9 is already out of support upstream, so I expect we will switch
> to GCC 5 during the next cycle.

Right, but it does not prevent up from adding a wrapper for current minimum
GCC supported (and it add overflow wrapper could be used not only in
this specific patch).

> 
>>>>> +  /* Use a small buffer to combine processing of multiple characters.
>>>>> +     CONVERT_FROM_OTHER_STRING expects the buffer size in (wide)
>>>>> +     characters, and buf_length counts that.  */
>>>>> +#ifdef COMPILE_WPRINTF
>>>>> +  enum { buf_length = 64 };
>>>>> +  wchar_t buf[buf_length];
>>>>> +#else
>>>>> +  enum { buf_length = 256 };
>>>>> +  char buf[buf_length];
>>>>> +  _Static_assert (sizeof (buf) > MB_LEN_MAX,
>>>>> +		  "buffer is large enough for a single multi-byte character");
>>>>> +#endif
>>>>
>>>> Why different 'buf_length' sizes if you using the required type for array
>>>> creation?
>>>
>>> I don't understand the question.  I want to avoid a large stack frame.
>>> Regarding the different constants, see the comment above.
>>
>> If the buffer must be computed as wide characters, why not defined it
>> using wchar_t regardless:
>>
>>   wchar_t wbuf[64];
>>   const size_t buf_length = sizeof (wbuf) / sizeof (CHAR_T); 
>>
>> ?
> 
> Well, it would have to be like this:
> 
>   enum { buf_length = 256 / sizeof (CHAR_T) };
>   CHAR_T buf[buf_length];
> 
> I think this should work.
> 
> (I've already posted a test which adds coverage for this code and would
> check that in first.)

Right, I forgot about the type requirement.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]