Date: Mon, 19 Apr 2010 11:01:40 -0500 From: Robert Noland <rnoland@FreeBSD.org> To: Luigi Rizzo <luigi@FreeBSD.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r206843 - head/sbin/ipfw Message-ID: <4BCC7E64.4040200@FreeBSD.org> In-Reply-To: <201004191511.o3JFBjLj036350@svn.freebsd.org> References: <201004191511.o3JFBjLj036350@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Luigi Rizzo wrote: > Author: luigi > Date: Mon Apr 19 15:11:45 2010 > New Revision: 206843 > URL: http://svn.freebsd.org/changeset/base/206843 > > Log: > Slightly different handling of printf/snprintf for unaligned uint64_t, > which should improve readability, and also to ease the port to > platforms that do not support %llu > > MFC after: 3 days > > Modified: > head/sbin/ipfw/dummynet.c > head/sbin/ipfw/ipfw2.c > head/sbin/ipfw/ipfw2.h > > Modified: head/sbin/ipfw/dummynet.c > ============================================================================== > --- head/sbin/ipfw/dummynet.c Mon Apr 19 14:34:44 2010 (r206842) > +++ head/sbin/ipfw/dummynet.c Mon Apr 19 15:11:45 2010 (r206843) > @@ -203,9 +203,9 @@ list_flow(struct dn_flow *ni) > inet_ntop(AF_INET6, &(id->dst_ip6), buff, sizeof(buff)), > id->dst_port); > } > - printf("%4llu %8llu %2u %4u %3u\n", > - align_uint64(&ni->tot_pkts), > - align_uint64(&ni->tot_bytes), > + pr_u64(&ni->tot_pkts, 4); > + pr_u64(&ni->tot_bytes, 8); > + printf("%2u %4u %3u\n", > ni->length, ni->len_bytes, ni->drops); > } > > @@ -290,8 +290,8 @@ static void > list_pipes(struct dn_id *oid, struct dn_id *end) > { > char buf[160]; /* pending buffer */ > - buf[0] = '\0'; > > + buf[0] = '\0'; > for (; oid != end; oid = O_NEXT(oid, oid->len)) { > if (oid->len < sizeof(*oid)) > errx(1, "invalid oid len %d\n", oid->len); > > Modified: head/sbin/ipfw/ipfw2.c > ============================================================================== > --- head/sbin/ipfw/ipfw2.c Mon Apr 19 14:34:44 2010 (r206842) > +++ head/sbin/ipfw/ipfw2.c Mon Apr 19 15:11:45 2010 (r206843) > @@ -314,22 +314,27 @@ static struct _s_x rule_options[] = { > { NULL, 0 } /* terminator */ > }; > > -/* > - * The following is used to generate a printable argument for > - * 64-bit numbers, irrespective of platform alignment and bit size. > - * Because all the printf in this program use %llu as a format, > - * we just return an unsigned long long, which is larger than > - * we need in certain cases, but saves the hassle of using > - * PRIu64 as a format specifier. > - * We don't care about inlining, this is not performance critical code. > +/* > + * Helper routine to print a possibly unaligned uint64_t on > + * various platform. If width > 0, print the value with > + * the desired width, followed by a space; > + * otherwise, return the required width. > */ > -unsigned long long > -align_uint64(const uint64_t *pll) > +int > +pr_u64(uint64_t *pd, int width) > { > - uint64_t ret; > +#ifdef TCC > +#define U64_FMT "I64" > +#else > +#define U64_FMT "llu" This is broken on amd64. It either needs casting or the following patch... beastie% svn diff Index: sbin/ipfw/ipfw2.c =================================================================== --- sbin/ipfw/ipfw2.c (revision 206844) +++ sbin/ipfw/ipfw2.c (working copy) @@ -326,7 +326,7 @@ #ifdef TCC #define U64_FMT "I64" #else -#define U64_FMT "llu" +#define U64_FMT "ju" #endif uint64_t d; robert. > +#endif > + uint64_t d; > > - bcopy (pll, &ret, sizeof(ret)); > - return ret; > + bcopy (pd, &d, sizeof(d)); > + return (width > 0) ? > + printf("%*" U64_FMT " ", width, d) : > + snprintf(NULL, 0, "%" U64_FMT, d) ; > +#undef U64_FMT > } > > void * > @@ -973,9 +978,10 @@ show_ipfw(struct ip_fw *rule, int pcwidt > } > printf("%05u ", rule->rulenum); > > - if (pcwidth>0 || bcwidth>0) > - printf("%*llu %*llu ", pcwidth, align_uint64(&rule->pcnt), > - bcwidth, align_uint64(&rule->bcnt)); > + if (pcwidth > 0 || bcwidth > 0) { > + pr_u64(&rule->pcnt, pcwidth); > + pr_u64(&rule->bcnt, bcwidth); > + } > > if (co.do_time == 2) > printf("%10u ", rule->timestamp); > @@ -1577,10 +1583,12 @@ show_dyn_ipfw(ipfw_dyn_rule *d, int pcwi > } > bcopy(&d->rule, &rulenum, sizeof(rulenum)); > printf("%05d", rulenum); > - if (pcwidth>0 || bcwidth>0) > - printf(" %*llu %*llu (%ds)", pcwidth, > - align_uint64(&d->pcnt), bcwidth, > - align_uint64(&d->bcnt), d->expire); > + if (pcwidth > 0 || bcwidth > 0) { > + printf(" "); > + pr_u64(&d->pcnt, pcwidth); > + pr_u64(&d->bcnt, bcwidth); > + printf("(%ds)", d->expire); > + } > switch (d->dyn_type) { > case O_LIMIT_PARENT: > printf(" PARENT %d", d->count); > @@ -1645,9 +1653,9 @@ ipfw_sets_handler(char *av[]) > free(data); > nalloc = nalloc * 2 + 200; > nbytes = nalloc; > - data = safe_calloc(1, nbytes); > - if (do_cmd(IP_FW_GET, data, (uintptr_t)&nbytes) < 0) > - err(EX_OSERR, "getsockopt(IP_FW_GET)"); > + data = safe_calloc(1, nbytes); > + if (do_cmd(IP_FW_GET, data, (uintptr_t)&nbytes) < 0) > + err(EX_OSERR, "getsockopt(IP_FW_GET)"); > } > > bcopy(&((struct ip_fw *)data)->next_rule, > @@ -1837,14 +1845,12 @@ ipfw_list(int ac, char *av[], int show_c > continue; > > /* packet counter */ > - width = snprintf(NULL, 0, "%llu", > - align_uint64(&r->pcnt)); > + width = pr_u64(&r->pcnt, 0); > if (width > pcwidth) > pcwidth = width; > > /* byte counter */ > - width = snprintf(NULL, 0, "%llu", > - align_uint64(&r->bcnt)); > + width = pr_u64(&r->bcnt, 0); > if (width > bcwidth) > bcwidth = width; > } > @@ -1858,13 +1864,11 @@ ipfw_list(int ac, char *av[], int show_c > if (set != co.use_set - 1) > continue; > } > - width = snprintf(NULL, 0, "%llu", > - align_uint64(&d->pcnt)); > + width = pr_u64(&d->pcnt, 0); > if (width > pcwidth) > pcwidth = width; > > - width = snprintf(NULL, 0, "%llu", > - align_uint64(&d->bcnt)); > + width = pr_u64(&d->bcnt, 0); > if (width > bcwidth) > bcwidth = width; > } > > Modified: head/sbin/ipfw/ipfw2.h > ============================================================================== > --- head/sbin/ipfw/ipfw2.h Mon Apr 19 14:34:44 2010 (r206842) > +++ head/sbin/ipfw/ipfw2.h Mon Apr 19 15:11:45 2010 (r206843) > @@ -207,7 +207,7 @@ enum tokens { > #define NEED(_p, msg) {if (!_p) errx(EX_USAGE, msg);} > #define NEED1(msg) {if (!(*av)) errx(EX_USAGE, msg);} > > -unsigned long long align_uint64(const uint64_t *pll); > +int pr_u64(uint64_t *pd, int width); > > /* memory allocation support */ > void *safe_calloc(size_t number, size_t size);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4BCC7E64.4040200>