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

[Stefan Liebler]

Hi Florian,

i get a build error on s390x:
gcc scratch_buffer_grow_preserve.c -c
...
scratch_buffer_grow_preserve.c: In function 
‘__libc_scratch_buffer_grow_preserve’:
scratch_buffer_grow_preserve.c:35:7: error: implicit declaration of 
function ‘memcpy’ [-Werror=implicit-function-declaration]
        memcpy (new_ptr, buffer->__space, buffer->length);
        ^
scratch_buffer_grow_preserve.c:35:7: error: incompatible implicit 
declaration of built-in function ‘memcpy’ [-Werror]
cc1: all warnings being treated as errors

#include <string.h> in malloc/scratch_buffer_grow_preserve.c solves this 
error. Please update scratch_buffer_grow_preserve.c.

Bye
Stefan

On 04/07/2015 11:10 AM, Florian Weimer wrote:
> 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).
>