Skip site navigation (1)Skip section navigation (2)
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>