From owner-freebsd-current@FreeBSD.ORG Tue Aug 24 22:17:31 2004 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id C645216A4D9; Tue, 24 Aug 2004 22:17:31 +0000 (GMT) Received: from mail.mcneil.com (rrcs-west-24-199-45-54.biz.rr.com [24.199.45.54]) by mx1.FreeBSD.org (Postfix) with ESMTP id 7128043D1F; Tue, 24 Aug 2004 22:17:29 +0000 (GMT) (envelope-from sean@mcneil.com) Received: from localhost (localhost.mcneil.com [127.0.0.1]) by mail.mcneil.com (Postfix) with ESMTP id 05866FD02A; Tue, 24 Aug 2004 15:17:29 -0700 (PDT) Received: from mail.mcneil.com ([127.0.0.1]) by localhost (server.mcneil.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 00574-09; Tue, 24 Aug 2004 15:17:28 -0700 (PDT) Received: from [24.199.45.54] (mcneil.com [24.199.45.54]) by mail.mcneil.com (Postfix) with ESMTP id 62C68FD026; Tue, 24 Aug 2004 15:17:28 -0700 (PDT) From: Sean McNeil To: Harti Brandt In-Reply-To: <1093377802.43728.11.camel@server.mcneil.com> References: <1092777586.92327.9.camel@server.mcneil.com> <20040817213813.GE3827@gothmog.gr><4127841D.6050104@freebsd.org> <1093369220.10362.6.camel@server.mcneil.com> <20040824200225.V517@beagle.kn.op.dlr.de> <1093371600.10464.23.camel@server.mcneil.com> <20040824205417.L517@beagle.kn.op.dlr.de> <1093377802.43728.11.camel@server.mcneil.com> Content-Type: text/plain Message-Id: <1093385848.84723.2.camel@server.mcneil.com> Mime-Version: 1.0 X-Mailer: Ximian Evolution 1.4.6 Date: Tue, 24 Aug 2004 15:17:28 -0700 Content-Transfer-Encoding: 7bit X-Virus-Scanned: by amavisd-new at mcneil.com cc: Giorgos Keramidas cc: Tim Kientzle cc: current@freebsd.org Subject: Re: bsdtar core dumps X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 24 Aug 2004 22:17:32 -0000 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