Re: [PATCH v2] nss: Get rid of alloca usage in makedb's write_output.

[Arjun Shankar <arjun@redhat.com>]

Hi Joe,

> Replace alloca usage with a scratch_buffer.
> ---
> Changes to v1:
>  * move scratch_buffer_free after the call to error ().
>
>  nss/makedb.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/nss/makedb.c b/nss/makedb.c
> index 48c8fe1333..67c3c4c893 100644
> --- a/nss/makedb.c
> +++ b/nss/makedb.c
> @@ -25,6 +25,7 @@
>  #include <inttypes.h>
>  #include <libintl.h>
>  #include <locale.h>
> +#include <scratch_buffer.h>
>  #include <search.h>
>  #include <stdbool.h>
>  #include <stdio.h>

OK.

> @@ -739,7 +740,16 @@ write_output (int fd)
>    struct nss_db_header *header;
>    uint64_t file_offset = (sizeof (struct nss_db_header)
>                           + (ndatabases * sizeof (header->dbs[0])));
> -  header = alloca (file_offset);
> +  struct scratch_buffer sbuf;
> +  scratch_buffer_init (&sbuf);
> +
> +
> +  if (!scratch_buffer_set_array_size (&sbuf, 1, file_offset))
> +    {
> +      error (0, errno, gettext ("failed to allocate memory"));
> +      return EXIT_FAILURE;
> +    }

The way scratch_buffer_init and scratch_buffer_set_array_size are
implemented currently, a failure of the set_array_size here looks like
it doesn't necessarily need a scratch_buffer_free to be called after
failure because there's nothing to be freed. But I think calling
scratch_buffer_free here before returning here is the safer bet.

> +  header = sbuf.data;
>
>    header->magic = NSS_DB_MAGIC;
>    header->ndbs = ndatabases;
> @@ -803,6 +813,7 @@ write_output (int fd)
>    if (writev (fd, iov, iov_nelts) != keydataoffset)
>      {
>        error (0, errno, gettext ("failed to write new database file"));
> +      scratch_buffer_free (&sbuf);
>        return EXIT_FAILURE;
>      }

OK.

>
> @@ -810,6 +821,7 @@ write_output (int fd)
>    DIAG_POP_NEEDS_COMMENT;
>  #endif
>
> +  scratch_buffer_free (&sbuf);
>    return EXIT_SUCCESS;
>  }

OK.

-- 
Arjun Shankar
he/him/his