[PATCH 06/25] Add struct scratch_buffer and its internal helper functions

[Florian Weimer]

On 04/06/2015 11:54 PM, Paul Eggert wrote:
> Thanks, this looks good.  One performance nit that I noticed this time
> around.  here:
> 
> On 04/06/2015 10:01 AM, Florian Weimer wrote:
>> +  /* Avoid overflow check if both values are small. */
>> +  if ((nelem | size) >> (sizeof (size_t) * CHAR_BIT / 2) != 0)
>> +    {
>> +      if (nelem != 0 && size > SIZE_MAX / nelem)
>> +    {
>> +      /* Overflow.  Discard the old buffer, but it must remain
>> +         valid to free.  */
>> +      scratch_buffer_free (buffer);
>> +      scratch_buffer_init (buffer);
>> +      __set_errno (ENOMEM);
>> +      return false;
>> +    }
>> +    }
>> +
>> +  size_t new_length = nelem * size;
> 
> It's better to compute new_length first, before the overflow check, and
> then replace "size > SIZE_MAX / nelem" with "size != new_length /
> nelem".  This will avoid the need for having SIZE_MAX in the executable,
> which should shrink the code a tad.  Also, hardware integer division
> tends to run faster with smaller dividends, which would be the case if
> this change were made.

Thanks for the suggestion.  I incorporated it into the version I
committed after verifying it does indeed reduce code size (also attached).

-- 
Florian Weimer / Red Hat Product Security
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Add-struct-scratch_buffer-and-its-internal-helper-fu.patch
Type: text/x-patch
Size: 19182 bytes
Desc: not available
URL: <http://sourceware.org/pipermail/libc-alpha/attachments/20150407/e5635b94/attachment.bin>