This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] vfprintf: Remove alloca from string formatting with conversion
On 28/06/2017 16:45, Florian Weimer wrote:
> On 06/28/2017 09:33 PM, Adhemerval Zanella wrote:
>>
>>
>> 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).
>
> The overflow check you posted assumes that left is positive. A generic
> signed overflow check is not easy to write in portable C. It's possible
> to get GCC to generate reasonably efficient code (even without -fwrapv),
> but you need one GCC extension (casts from unsigned int to int do not
> alter the bit pattern). Even then, the generated code would be worse
> than what the current vfprintf-specific check does.
Yes, this was just an example for the specific case in question, maybe
renaming it to explicit assume left is positive or returning an error
for specific non supported case. gnulib has as workable example using
macros for general case (intprops.h), but I would rather avoid and using
explicit typed inline functions (as we did for size_t overflow). And for
these specific cases I would not focus on performance, but rather an API
which explicit states its idea (avoid overflow), performance would be
mainly obtained though GCC builtins used conditionally.