This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [3.3/mainline;libiberty] Fix vasprintf.c


If it is an array, isn't passing by value and THEN copying
it (again) a waste of cpu time?

It seems that you should leave the 'va_list *args;'
in the prototype of int_vasprintf.

For example,

#ifndef HAVE_VA_COPY
#include <stdarg.h>
#ifndef va_copy
#ifdef VA_LIST_IS_STRUCT
#define va_copy(dest, src) (dest) = (src)
#else
#define va_copy(dest, src) *(dest) = *(src)
#endif
#endif /* !va_copy */
#endif /* !HAVE_VA_COPY */

static int
int_vasprintf (result, format, args)
     char **result;
     const char *format;
     va_list *args;
{
[...]
  va_copy (ap, *args);


The 'VA_LIST_IS_STRUCT' test is needed because on some
compilers it is possible that va_copy is not a macro or
function, but still va_list is not a pointer but is a
structure.

I used the following as a configure test for that in
ircu:

dnl
dnl Macro: ircu_VA_LIST
dnl
dnl Check if va_list is a struct or an array/pointer.
dnl
AC_DEFUN(ircu_VA_LIST,
[dnl Check if va_list is a struct or an array/pointer.
AC_CACHE_CHECK([if va_list is a struct], ircu_cv_sys_va_list_is_struct,
[AC_TRY_RUN([#include <stdarg.h>
void vtest(va_list vl)
{
  va_arg(vl, int);
}
int test(int ret, ...)
{
  va_list vl;
  va_start(vl, ret);
  vtest(vl);
  ret = va_arg(vl, int);
  va_end(vl);
  return ret;
}
int main(void)
{
  exit(test(0, 0, 1));
}], ircu_cv_sys_va_list_is_struct=yes, ircu_cv_sys_va_list_is_struct=no)])
if test $ircu_cv_sys_va_list_is_struct = yes; then
  AC_DEFINE(VA_LIST_IS_STRUCT)
fi])

As soon as my future papers are done (should be within one or two
weeks, they are already being mailed to me), you can use this without
problems as I am the original author of it.

Also note that in the above I didn't use memcpy(), it depends
on the size of va_list if that is efficient or not.  By using
just #define va_copy(dest, src) (dest) = (src), gcc will make
that decision by itself.

Regards,

Carlo Wood

On Fri, Oct 03, 2003 at 01:15:44PM +0200, Josef Zlomek wrote:
> Hello,
> 
> this patch fixes vasprintf.c. The problem was that on some architectures
> (x86-64 and ppc64) va_list is not a pointer but an array.
> 
> Tested on x86-64, ppc32, ppc64, ia32, ia64, alpha, sparc32, sparc64
> (vasprintf.c contains a test enabled by -DTEST)
> Bootstrapped/regtested x86=64.
> 
> OK for mainline and 3.3 branch?
> 
> Josef
> 
> 2003-10-03  Josef Zlomek  <zlomekj@suse.cz>
> 
> 	Jan Hubicka <jh@suse.cz>
> 	* vasprintf.c (int_vasprintf): Pass va_list by value.
> 	Use va_copy for copying va_list.
> 	(vasprintf): Pass va_list by value.
> 
> Index: vasprintf.c
> ===================================================================
> RCS file: /cvs/gcc/gcc/libiberty/vasprintf.c,v
> retrieving revision 1.12
> diff -c -3 -p -r1.12 vasprintf.c
> *** vasprintf.c	3 Jun 2003 18:19:17 -0000	1.12
> --- vasprintf.c	2 Oct 2003 20:07:33 -0000
> *************** not be allocated, minus one is returned 
> *** 59,71 ****
>   
>   */
>   
> ! static int int_vasprintf PARAMS ((char **, const char *, va_list *));
>   
>   static int
>   int_vasprintf (result, format, args)
>        char **result;
>        const char *format;
> !      va_list *args;
>   {
>     const char *p = format;
>     /* Add one to make sure that it is never zero, which might cause malloc
> --- 59,71 ----
>   
>   */
>   
> ! static int int_vasprintf PARAMS ((char **, const char *, va_list));
>   
>   static int
>   int_vasprintf (result, format, args)
>        char **result;
>        const char *format;
> !      va_list args;
>   {
>     const char *p = format;
>     /* Add one to make sure that it is never zero, which might cause malloc
> *************** int_vasprintf (result, format, args)
> *** 73,79 ****
>     int total_width = strlen (format) + 1;
>     va_list ap;
>   
> !   memcpy ((PTR) &ap, (PTR) args, sizeof (va_list));
>   
>     while (*p != '\0')
>       {
> --- 73,83 ----
>     int total_width = strlen (format) + 1;
>     va_list ap;
>   
> ! #ifdef va_copy
> !   va_copy (ap, args);
> ! #else
> !   memcpy ((PTR) &ap, (PTR) &args, sizeof (va_list));
> ! #endif
>   
>     while (*p != '\0')
>       {
[cut]


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]