From owner-freebsd-audit Tue May 28 2:12:29 2002 Delivered-To: freebsd-audit@freebsd.org Received: from mailman.zeta.org.au (mailman.zeta.org.au [203.26.10.16]) by hub.freebsd.org (Postfix) with ESMTP id 84C6237B400 for ; Tue, 28 May 2002 02:12:16 -0700 (PDT) Received: from bde.zeta.org.au (bde.zeta.org.au [203.2.228.102]) by mailman.zeta.org.au (8.9.3/8.8.7) with ESMTP id TAA29085; Tue, 28 May 2002 19:12:07 +1000 Date: Tue, 28 May 2002 19:15:23 +1000 (EST) From: Bruce Evans X-X-Sender: bde@gamplex.bde.org To: Dima Dorfman Cc: Dag-Erling Smorgrav , Subject: Re: %j for printf(9) In-Reply-To: <20020528001615.4BAC93E5E@turbine.trit.org> Message-ID: <20020528184446.W19885-100000@gamplex.bde.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG On Tue, 28 May 2002, Dima Dorfman wrote: > Dag-Erling Smorgrav wrote: > > Dima Dorfman writes: > > > Attached is a patch that implements the %j length modifier in > > > printf(9). > > > > Here's an alternative (IMHO less disruptive) patch, which also fixes > > the default case. > > I still like my restructure, but since other people don't seem to > share my opinion, I'm fine doing it another (your) way. That said, I > think your patch has some bugs that mine doesn't. For example, this: > > printf("%ld\n", -4); > > yields "4294967292" with your patch, but not with mine (mine, and > printf(3), yield "-4"). I think the attached patch (relative to > subr_prf.c *with* your patch applied) fixes it. Both are not incorrect, since the behaviour is undefined :-). printf("%ld", -4L) would be a better example. My objections to your restructuring are mostly related to getting the conversions for this and other things things right. Forcing everything into a uintmax_t isn't really simple or correct, and the restructuring depends on it. > +++ subr_prf.c Tue May 28 00:09:30 2002 > @@ -658,19 +658,19 @@ > if (jflag) > num = va_arg(ap, uintmax_t); > else if (qflag) > - num = va_arg(ap, u_quad_t); > + num = (u_quad_t)va_arg(ap, u_quad_t); > else if (lflag) > - num = va_arg(ap, u_long); > + num = (u_long)va_arg(ap, u_long); > else > - num = va_arg(ap, u_int); > + num = (u_int)va_arg(ap, u_int); These casts are all no-ops. va_arg() already gives the correct type. This type is unsigned, so there are no surprises promoting it to the type of `num' (uintmax_t). > goto nosign; > fetch_number: > if (jflag) > - num = va_arg(ap, uintmax_t); > + num = va_arg(ap, intmax_t); An (explicit but unnecessary) cast to uintmax_t would make some sense here, since we are changing the value of an intmax_t to store it into a uintmax_t. The conversion happens by default but only clearly works right on 2's complement machines. > else if (qflag) > - num = va_arg(ap, u_quad_t); > + num = (quad_t)va_arg(ap, quad_t); This cast also has no effect. Changing the type in the va_arg() also has no effect on any supported machine, but is necessary on machines with quad_t smaller than intmax_t. Explicit but null casts, first to intmax_t and then to uintmax_t, might be good to show what is happening here. > else if (lflag) > - num = va_arg(ap, u_long); > + num = (long)va_arg(ap, long); u_long was wrong in the same way as u_quad_t was wrong, except the wrongness actually affects supported machines (ones with long smaller than intmax_t). > else > num = sign ? (uintmax_t)va_arg(ap, int) : > va_arg(ap, u_int); I think this `sign' test is needed for the lflag and qflag cases too. It is needed to get the correct sign extension for promotion of types smaller than `num'. In rev.1.1, `int' was the only such type, but in theory even long longs might be smaller that intmax_t. Support for machines with longs smaller than quads seems to be broken in -current. This only affects the %r and %z formats. Bruce To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message