Re: [PATCH] nss_files: Use struct scratch_buffer for gethostbyname [BZ #18023]

[Florian Weimer <fweimer at redhat dot com>]

On 09/05/2017 07:53 PM, Adhemerval Zanella wrote:

>>  /* Get implementation for some internal functions.  */
>> @@ -121,15 +122,12 @@ gethostbyname3_multi (FILE * stream, const char *name, int af,
>>  		      int *errnop, int *herrnop, int flags)
>>  {
>>    /* We have to get all host entries from the file.  */
>> -  size_t tmp_buflen = MIN (buflen, 4096);
>> -  char tmp_buffer_stack[tmp_buflen]
>> -    __attribute__ ((__aligned__ (__alignof__ (struct hostent_data))));
> 
> I can't really tell how important is the alignment of this buffer in particular,
> since on subsequent 'internal_getent' it does uses a plain char buffer.  Do we
> need to keep the alignment of this buffer?

internal_getent must align on its own because it is called with user
buffers and POSIX doesn't say anything about the alignment.

But struct scratch_buffer supplies max_align_t alignment anyway, which
is at least the alignment of struct hostent_data (by definition).

> I do think this it is easier to read and follow the code *without* the goto,
> something like:
> 
> scratch_buffer_init (...);
> while (1)
>   {
>     while ((status = internal_getent (...)) == NSS_STATUS_SUCCESS)
>       {
>         ...
>       }
>     if (status == NSS_STATUS_TRYAGAIN)
>       if (!scratch_buffer_grow (&tmp_buffer))
>         {
>           *herrnop = NETDB_INTERNAL;
>           status = NSS_STATUS_TRYAGAIN;
>           break;
>         }
>     else
>       status = NSS_STATUS_SUCCESS;
>   }
> scratch_buffer_free (...);

Right, I think I'll make this change in the first (refactoring) patch.

Thanks,
Florian