[3/6] vfprintf: Define WORK_BUFFER_SIZE
Commit Message
This constant will allow us to refer to the number of elements in
work_buffer across a function call boundary.
---
stdio-common/vfprintf.c | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)
Comments
Florian Weimer wrote:
> - CHAR_T work_buffer[1000];
> +#define WORK_BUFFER_SIZE 1000
> + CHAR_T work_buffer[WORK_BUFFER_SIZE];
Another nit: I suggest avoiding the macro, as it's confusing when #defined
inside a function body but intended to be used outside the function, and instead
doing this at the top level:
enum { WORK_BUFFER_SIZE = 1000 };
The general idea is to use a macro only when necessary.
On 03/03/2015 03:07 AM, Paul Eggert wrote:
> Florian Weimer wrote:
>> - CHAR_T work_buffer[1000];
>> +#define WORK_BUFFER_SIZE 1000
>> + CHAR_T work_buffer[WORK_BUFFER_SIZE];
>
> Another nit: I suggest avoiding the macro, as it's confusing when
> #defined inside a function body but intended to be used outside the
> function, and instead doing this at the top level:
>
> enum { WORK_BUFFER_SIZE = 1000 };
>
> The general idea is to use a macro only when necessary.
Good idea, thanks.
On 03/01/2015 03:52 PM, Florian Weimer wrote:
> This constant will allow us to refer to the number of elements in
> work_buffer across a function call boundary.
> ---
> stdio-common/vfprintf.c | 19 +++++++++----------
> 1 file changed, 9 insertions(+), 10 deletions(-)
Looks to me as long as you follow Paul's suggestion to
use an enum instead of a burried macro.
Cheers,
Carlos.
@@ -235,7 +235,8 @@ vfprintf (FILE *s, const CHAR_T *format, va_list ap)
const UCHAR_T *end_of_spec;
/* Buffer intermediate results. */
- CHAR_T work_buffer[1000];
+#define WORK_BUFFER_SIZE 1000
+ CHAR_T work_buffer[WORK_BUFFER_SIZE];
CHAR_T *workstart = NULL;
CHAR_T *workend;
@@ -986,7 +987,7 @@ vfprintf (FILE *s, const CHAR_T *format, va_list ap)
/* Print description of error ERRNO. */ \
string = \
(CHAR_T *) __strerror_r (save_errno, (char *) work_buffer, \
- sizeof work_buffer); \
+ WORK_BUFFER_SIZE * sizeof (CHAR_T)); \
is_long = 0; /* This is no wide-char string. */ \
goto LABEL (print_string)
@@ -1366,7 +1367,7 @@ vfprintf (FILE *s, const CHAR_T *format, va_list ap)
CHAR_T spec;
workstart = NULL;
- workend = &work_buffer[sizeof (work_buffer) / sizeof (CHAR_T)];
+ workend = work_buffer + WORK_BUFFER_SIZE;
/* Get current character in format string. */
JUMP (*++f, step0_jumps);
@@ -1465,7 +1466,7 @@ vfprintf (FILE *s, const CHAR_T *format, va_list ap)
goto all_done;
}
- if (width >= sizeof (work_buffer) / sizeof (work_buffer[0]) - 32)
+ if (width >= WORK_BUFFER_SIZE - 32)
{
/* We have to use a special buffer. The "32" is just a safe
bet for all the output which is not counted in the width. */
@@ -1498,7 +1499,7 @@ vfprintf (FILE *s, const CHAR_T *format, va_list ap)
goto all_done;
}
- if (width >= sizeof (work_buffer) / sizeof (work_buffer[0]) - 32)
+ if (width >= WORK_BUFFER_SIZE - 32)
{
/* We have to use a special buffer. The "32" is just a safe
bet for all the output which is not counted in the width. */
@@ -1564,8 +1565,7 @@ vfprintf (FILE *s, const CHAR_T *format, va_list ap)
}
else
prec = 0;
- if (prec > width
- && prec > sizeof (work_buffer) / sizeof (work_buffer[0]) - 32)
+ if (prec > width && prec > WORK_BUFFER_SIZE - 32)
{
if (__glibc_unlikely (prec >= INT_MAX / sizeof (CHAR_T) - 32))
{
@@ -1935,7 +1935,7 @@ do_positional:
CHAR_T spec = specs[nspecs_done].info.spec;
workstart = NULL;
- workend = &work_buffer[sizeof (work_buffer) / sizeof (CHAR_T)];
+ workend = work_buffer + WORK_BUFFER_SIZE;
/* Fill in last information. */
if (specs[nspecs_done].width_arg != -1)
@@ -1969,8 +1969,7 @@ do_positional:
}
/* Maybe the buffer is too small. */
- if (MAX (prec, width) + 32 > (int) (sizeof (work_buffer)
- / sizeof (CHAR_T)))
+ if (MAX (prec, width) + 32 > WORK_BUFFER_SIZE)
{
if (__libc_use_alloca ((MAX (prec, width) + 32)
* sizeof (CHAR_T)))