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: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20040307113008.GC64109>