Re: add xvasprintf and vasprintf to libiberty

[Manuel López-Ibáñez <lopezibanez at gmail dot com>]

2009/8/11 DJ Delorie <dj@redhat.com>:
>
> Index: libiberty/xvasprintf.c
> ===================================================================
> --- libiberty/xvasprintf.c ? ? ?(revision 0)
> +++ libiberty/xvasprintf.c ? ? ?(revision 0)
> @@ -0,0 +1,67 @@
> +/*
> +
> +@deftypefn Replacement char * xvasprintf (const char *@var{format}, va_list @var{args})
> +
> +Print to allocated string without fail. ?If @code{vasprintf} fails,
>
> vasprintf?

Would xmalloc be OK?

>
> +static char * int_xvasprintf (const char *, va_list);
> +
> +static char *
> +int_xvasprintf (const char *format, va_list args)
> +{
> + ?char * result;
> + ?int total_width = strlen_vprintf (format, args);
> + ?result = (char *) xmalloc (total_width);
> + ?vsprintf (result, format, args);
> + ?return result;
> +}
> +
> +char *
> +xvasprintf (const char *format,
> +#if defined (_BSD_VA_LIST_) && defined (__FreeBSD__)
> + ? ? ? ? ? _BSD_VA_LIST_ args)
> +#else
> + ? ? ? ? ? va_list args)
> +#endif
> +{
> + ?return int_xvasprintf (format, args);
> +}
>
> What's the point of having an internal function, if there's exactly
> one caller?

You should ask whoever implemented vasprintf.c. If you are happier
without the internal function, I will remove it.

> +static inline int strlen_vprintf (const char *, va_list);
> +
> +static inline int
> +strlen_vprintf (const char *format, va_list args)
> +{
>
> I'd rather avoid code-in-headers. ?The function name also makes me

Then it cannot be inline.

> think it's going to take the length of a string, then print it. ?How
> about vprintf_buffer_size_needed or something? ?If it's internal, it's
> OK to be verbose. ?Or we can call it "libiberty_vprintf_buffer_size"
> and put it in its own object.

Could you tell me what this entails? Do I need to add it as I did with
xvasprintf.c? Or is the process different for internal functions?
Would be enough to create a vprintf_buffer_size_needed.c and
vprintf_buffer_size_needed.h and add them to Makefile.in?

> + ?/* Add one to make sure that it is never zero, which might cause malloc
> + ? ? to return NULL. ?*/
> + ?int total_width = strlen (format) + 1;
>
> What about adding one for the final NUL byte?
>

Do you mean +2 or just changing the comment?

Cheers,

Manuel.

PS: Please CC me, I am not subscribed to gcc-patches anymore.