This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] vfprintf: Use struct scratch_buffer for positional arguments allocation
- From: Adhemerval Zanella <adhemerval dot zanella at linaro dot org>
- To: libc-alpha at sourceware dot org
- Date: Tue, 27 Jun 2017 15:07:04 -0300
- Subject: Re: [PATCH] vfprintf: Use struct scratch_buffer for positional arguments allocation
- Authentication-results: sourceware.org; auth=none
- References: <20170619161949.12053402AEC0E@oldenburg.str.redhat.com>
On 19/06/2017 13:19, Florian Weimer wrote:
> 2017-06-19 Florian Weimer <fweimer@redhat.com>
>
> * stdio-common/vfprintf.c (printf_positional): Use struct
> scratch_buffer to allocate backing storage for the args_value,
> args_size, args_type arrays.
>
> - /* Set up the remaining two arrays to each point past the end of the
> - prior array, since space for all three has been allocated now. */
> - args_size = &args_value[nargs].pa_int;
> - args_type = &args_size[nargs];
> - memset (args_type, s->_flags2 & _IO_FLAGS2_FORTIFY ? '\xff' : '\0',
> - nargs * sizeof (*args_type));
> + union printf_arg *args_value;
> + int *args_size;
> + int *args_type;
> + {
> + /* Calculate total size needed to represent a single argument
> + across all three argument-related arrays. */
> + size_t bytes_per_arg
> + = sizeof (*args_value) + sizeof (*args_size) + sizeof (*args_type);
LGTM, although I do not see much gain for using brackets just for
bytes_per_arg.
> + if (!scratch_buffer_set_array_size (&argsbuf, nargs, bytes_per_arg))
> + {
> + done = -1;
> + goto all_done;
> + }
> + args_value = argsbuf.data;
> + /* Set up the remaining two arrays to each point past the end of
> + the prior array, since space for all three has been allocated
> + now. */
> + args_size = &args_value[nargs].pa_int;
> + args_type = &args_size[nargs];
> + memset (args_type, s->_flags2 & _IO_FLAGS2_FORTIFY ? '\xff' : '\0',
> + nargs * sizeof (*args_type));
> + }
>
> /* XXX Could do sanity check here: If any element in ARGS_TYPE is
> still zero after this loop, format is invalid. For now we
> @@ -1966,8 +1955,7 @@ printf_positional (_IO_FILE *s, const CHAR_T *format, int readonly_format,
> - specs[nspecs_done].end_of_fmt);
> }
> all_done:
> - if (__glibc_unlikely (args_malloced != NULL))
> - free (args_malloced);
> + scratch_buffer_free (&argsbuf);
> scratch_buffer_free (&specsbuf);
> return done;
> }
>