where do %j/uintmax_t stand in terms of standards? [WAS: Re: WARNS cleanup for ipfw

[Luigi Rizzo]

On Sun, Mar 07, 2004 at 12:30:08PM +0100, Johan Karlsson wrote:
...
> Ok, how about the attached patch then? It takes care of all printf
> related warnings on -current.

not there yet, sorry...

No offense, but I think that rather than rushing for a commit you
should wait a few days to get some feedback from people using 64-bit
platforms (e.g.  try to post to -sparc or -alpha, or ask some of
the people involved with 64-bit development), and also have a look
at how other system utilities deal with similar things (64-bit
counters and possible alignment problems -- what is the preferred
way to print out things, "%qu" or "%llu" ? I understand that
ipfw2.c does a mix of both things, i just have no idea which one is
better except that "unsigned long long" is a lot longer to
write than "u_quad_t" so that might favour "%qu" ?).
In any case, it's a weekend, give people a bit of time to read and
think about solutions.

This is muddy ground, I and possibly others have burned our fingers
by making the wrong assumptions. A clean and silent compile
can still cause the code to dump core on certain systems due to
alignment problems.

E.g. I strongly suspect that, on systems with aligmnent issues,
many of the places where you cast values to (unsigned long long)
would be a lot safer by using align_uint64() (I believe the current
code _is_ safe because of the way the pointer to list_pipes() and
print_flowset_parms() are constructed, but all this is very fragile,
because it relies on the userland passing down a suitably aligned
buffer, which is not specified anywhere in the ipfw ABI,
If we are going to touch this code we better provide a good fix
than a bandaid.)

	cheers
	luigi

> 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 at FreeBSD.org

> 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);

> _______________________________________________
> freebsd-ipfw at freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-ipfw
> To unsubscribe, send any mail to "freebsd-ipfw-unsubscribe at freebsd.org"