Re: bsdtar core dumps

On Tue, 2004-08-24 at 12:03, Harti Brandt wrote:
> On Tue, 24 Aug 2004, Sean McNeil wrote:
> 
> SM>On Tue, 2004-08-24 at 11:08, Harti Brandt wrote:
> SM>> On Tue, 24 Aug 2004, Sean McNeil wrote:
> SM>> 
> SM>[snip]
> SM>> 
> SM>> Sorry to jump in.
> SM>> 
> SM>> You cannot use a va_list twice. As soon as someone call va_arg() on the
> SM>> ap all the aps in the calling functions get invalid. The only thing that 
> SM>> can and must be done is that the function that did the va_start() must 
> SM>> call va_end.
> SM>> 
> SM>> If you need it twice you must make a copy as in the patch below.
> SM>> But the function call va_copy must also call va_end() on that copy
> SM>> (this seems missing in the patch).
> SM>
> SM>I took the lead from /usr/src/lib/libc/stdio/vfprintf.c
> SM>
> SM>It happens to do a va_copy(orgap,ap) but never does a va_end(orgap). 
> SM>Either vfprintf.c is incorrect or it is not necessary to call va_end().
> 
> vfprintf.c is wrong. Paragraph 1 of 7.15.1 of C99 states: "Each invocation 
> of the va_start and va_copy macros shall be matched by a corresponding 
> invocation of the va_end macro in the same function."

C99 is good enough for me.  I suppose this is so that va_copy can be
implemented with some sort of allocation scheme.  Could someone commit
fixes to vfprintf.c and archive_string_sprintf.c?

*** lib/libarchive/archive_string_sprintf.c.orig        Fri Aug 13
20:45:45 2004--- lib/libarchive/archive_string_sprintf.c     Tue Aug 24
12:58:24 2004
*************** __archive_string_vsprintf(struct archive
*** 47,65 ****
      va_list ap)
  {
        size_t l;

        if (fmt == NULL) {
                as->s[0] = 0;
                return;
        }

        l = vsnprintf(as->s, as->buffer_length, fmt, ap);
        /* If output is bigger than the buffer, resize and try again. */
        if (l+1 >= as->buffer_length) {
                __archive_string_ensure(as, l + 1);
!               l = vsnprintf(as->s, as->buffer_length, fmt, ap);
        }
        as->length = l;
  }

  /*
--- 47,68 ----
      va_list ap)
  {
        size_t l;
+       va_list ap1;

        if (fmt == NULL) {
                as->s[0] = 0;
                return;
        }

+       va_copy(ap1,ap);
        l = vsnprintf(as->s, as->buffer_length, fmt, ap);
        /* If output is bigger than the buffer, resize and try again. */
        if (l+1 >= as->buffer_length) {
                __archive_string_ensure(as, l + 1);
!               l = vsnprintf(as->s, as->buffer_length, fmt, ap1);
        }
        as->length = l;
+       va_end(ap1);
  }

  /*

*** lib/libc/stdio/vfprintf.c.orig      Mon Aug 23 20:35:43 2004
--- lib/libc/stdio/vfprintf.c   Tue Aug 24 13:01:45 2004
*************** error:
*** 1237,1242 ****
--- 1237,1243 ----
        if (dtoaresult != NULL)
                freedtoa(dtoaresult);
  #endif
+       va_end(orgap);
        if (convbuf != NULL)
                free(convbuf);
        if (__sferror(fp))