Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 7 Mar 2004 12:30:08 +0100
From:      Johan Karlsson <johan@freebsd.org>
To:        Luigi Rizzo <rizzo@icir.org>
Cc:        ipfw@freebsd.org
Subject:   Re: where do %j/uintmax_t stand in terms of standards? [WAS: Re: WARNS cleanup for ipfw
Message-ID:  <20040307113008.GC64109@numeri.campus.luth.se>
In-Reply-To: <20040306212233.A56351@xorpc.icir.org>
References:  <20040306111922.GA64109@numeri.campus.luth.se> <20040306082625.B34490@xorpc.icir.org> <20040306173219.GB64109@numeri.campus.luth.se> <20040306212233.A56351@xorpc.icir.org>

Next in thread | Previous in thread | Raw E-Mail | Index | Archive | Help

--neYutvxvOLaeuPCA
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Sat, Mar 06, 2004 at 21:22 (-0800) +0000, Luigi Rizzo wrote:
> On Sat, Mar 06, 2004 at 06:32:19PM +0100, Johan Karlsson wrote:
> > On Sat, Mar 06, 2004 at 08:26 (-0800) +0000, Luigi Rizzo wrote:
> > > On Sat, Mar 06, 2004 at 12:19:22PM +0100, Johan Karlsson wrote:
> > > > Hi
> > > > 
> > > > the attached patch makes ipfw WARNS=2 clean by using the
> > > > %j/(uintmax_t) combo where so needed. If there are no
> > > > objections I intend to commit this patch.
> > 
> > First of all, %j/uintmax_t is used since uint64_t does not match
> > long long on all our platforms. Hence to print this without warning
> > we need to do this.
> 
> ok, given that our counters _are_ 64 bits on all platforms, and
> that it would be nice to use the same code on 4.x and 5.x (at least,
> I'd hate to see a large number of differences for something trivial
> as a printf specifier), i suggest to leave the print format as "%llu",
> which is supported on all versions and platforms, and change
> align_uint64() as following:
> 
> -static __inline uint64_t
> +static unsigned long long
>  align_uint64(uint64_t *pll) {
> 	uint64_t ret;
> 
> +	/* make sure the value is correctly aligned, as pll might be not */
> 	bcopy (pll, &ret, sizeof(ret));
> -	return ret;
> +	return (unsigned long long)ret;
>  };
> 
> (we do not care about __inline since this is always called 
> within a *printf() which is way more expensive).
> This should close the issue, and is a lot more readable and
> portable than the proposed patch or my previous suggestion.

Ok, how about the attached patch then? It takes care of all printf
related warnings on -current.

I do not have a -stable machine at the moment so I have not done any
compile testing for -stable. If you agree to this patch, please commit
it or let me know if I should.

take care
/Johan K

-- 
Johan Karlsson		mailto:johan@FreeBSD.org

--neYutvxvOLaeuPCA
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="ipfw.diff2"

Index: sbin/ipfw/Makefile
===================================================================
RCS file: /home/ncvs/src/sbin/ipfw/Makefile,v
retrieving revision 1.12
diff -u -r1.12 Makefile
--- sbin/ipfw/Makefile	11 Jul 2002 17:33:37 -0000	1.12
+++ sbin/ipfw/Makefile	5 Mar 2004 22:06:10 -0000
@@ -2,7 +2,7 @@
 
 PROG=	ipfw
 SRCS=	ipfw2.c
-WARNS?=	0
+WARNS?=	2
 MAN=	ipfw.8
 
 .include <bsd.prog.mk>
Index: sbin/ipfw/ipfw2.c
===================================================================
RCS file: /home/ncvs/src/sbin/ipfw/ipfw2.c,v
retrieving revision 1.45
diff -u -r1.45 ipfw2.c
--- sbin/ipfw/ipfw2.c	24 Jan 2004 19:20:09 -0000	1.45
+++ sbin/ipfw/ipfw2.c	7 Mar 2004 11:12:34 -0000
@@ -352,12 +352,12 @@
 	{ NULL, 0 }	/* terminator */
 };
 
-static __inline uint64_t
+static unsigned long long
 align_uint64(uint64_t *pll) {
 	uint64_t ret;
 
 	bcopy (pll, &ret, sizeof(ret));
-	return ret;
+	return (unsigned long long)ret;
 };
 
 /*
@@ -1423,12 +1423,14 @@
 		ina.s_addr = htonl(q[l].id.dst_ip);
 		printf("%15s/%-5d ",
 		    inet_ntoa(ina), q[l].id.dst_port);
-		printf("%4qu %8qu %2u %4u %3u\n",
-		    q[l].tot_pkts, q[l].tot_bytes,
+		printf("%4llu %8llu %2u %4u %3u\n",
+		    (unsigned long long)q[l].tot_pkts,
+		    (unsigned long long)q[l].tot_bytes,
 		    q[l].len, q[l].len_bytes, q[l].drops);
 		if (verbose)
-			printf("   S %20qd  F %20qd\n",
-			    q[l].S, q[l].F);
+			printf("   S %20llu  F %20llu\n",
+			    (unsigned long long)q[l].S,
+			    (unsigned long long)q[l].F);
 	}
 }
 
@@ -1517,7 +1519,8 @@
 		    p->pipe_nr, buf, p->delay);
 		print_flowset_parms(&(p->fs), prefix);
 		if (verbose)
-			printf("   V %20qd\n", p->V >> MY_M);
+			printf("   V %20llu\n",
+			    (unsigned long long)p->V >> MY_M);
 
 		q = (struct dn_flow_queue *)(p+1);
 		list_queues(&(p->fs), q);

--neYutvxvOLaeuPCA--



Want to link to this message? Use this URL: <http://docs.FreeBSD.org/cgi/mid.cgi?20040307113008.GC64109>