bsdtar core dumps
Sean McNeil
sean at mcneil.com
Tue Aug 24 15:17:32 PDT 2004
On Tue, 2004-08-24 at 13:03, Sean McNeil wrote:
> 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))
I missed one in vfwprintf.c. I needs a va_end(orgap) just like above.
Also, I've searched all of src and found one additional place that there
is a va_copy without a va_end.
contrib/gnu-sort/lib/version-etc.c
All other uses appear to be properly matched.
Cheers,
Sean
More information about the freebsd-current
mailing list