Subject: Re: lib/16518 (some fixes to libpcap)
To: None <lib-bug-people@netbsd.org, gnats-admin@netbsd.org,>
From: Matthias Drochner <M.Drochner@fz-juelich.de>
List: netbsd-bugs
Date: 02/27/2006 13:35:02
The following reply was made to PR lib/16518; it has been noted by GNATS.

From: Matthias Drochner <M.Drochner@fz-juelich.de>
To: YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp>
Cc: gnats-bugs@NetBSD.org, lib-bug-people@NetBSD.org,
	netbsd-bugs@NetBSD.org, gnats-admin@NetBSD.org
Subject: Re: lib/16518 (some fixes to libpcap) 
Date: Mon, 27 Feb 2006 14:34:10 +0100

 yamt@mwd.biglobe.ne.jp said:
 > was not sure which of signed or unsigned should be used.
 
 For me it is pretty obvious that unsigned should be used,
 to be consistent with kernel behaviour. While the manpage
 doesn't tell, the kernel implementations are there and
 cannot be changed anymore, just documented.
 
 > see "Re: some fixes for complier and optimizer in libpcap" mails
 > on tcpdump-workers around June 2002
 
 The issues broght up there are unrelated afaict: Your patch
 is just about the operators in arithmetics. It doesn't
 affect comparisions. (The wrong optimisation of "1>1" was
 fixed by another patch which went into mainstream sources.
 And why ">"/"<" are not optimized rather than correctly optimized
 now is also another issue. "=" is optimized appearently, see below.)
 And assignments from singed to unsigned int32 values don't
 change anything -- that's more a "lint" issue.
 
 Just to illustrate things a bit:
 The filter expression "(0x80000000 / 2) > 0x40000000", if not
 optimized, leads to code which rejects everything, because the
 kernel evaluates this to "0x40000000 > 0x40000000".
 With (unpatched) optimization, the resulting program is:
 (000) ld       #0xc0000000
 (001) ldx      #0x40000000
 (002) jgt      x                jt 3    jf 4
 (003) ret      #96
 (004) ret      #0
 which accepts everything.
 Optimization of "=" gets a bit further -- a call of
 tcpdump "(0x80000000 / 2) = 0x40000000"
 does yield
 tcpdump: expression rejects all packets
 while
 tcpdump -O "(0x80000000 / 2) = 0x40000000"
 happily receives packets.
 
 Your patch fixes exactly these inconsistencies.
 Just to be sure, I'm talking about
 @@ -624,7 +624,7 @@ fold_op(s, v0, v1)
         struct stmt *s;
         int v0, v1;
  {
 -       bpf_int32 a, b;
 +       bpf_u_int32 a, b;
 
         a = vmap[v0].const_val;
         b = vmap[v1].const_val;
 
 > i guess it's better to bring a discussion on tcpdump-workers again
 
 This might be a good idea. Do you want to defent your patch once more?
 I'm just working on an update of NetBSD's libpcap to 0.9.4, and I plan
 to post the more essential changes to tcpdump-workers too.
 (Which is merely some "const" consistency for now, and support for
 the cloning /dev/bpf.)
 
 > try to fix the problem in their repository first
 
 We have local changes anyway, so I wouldn't wait if it is fixing
 a bug. And an optimizer changing the effect of code is a bug.
 (Even if only practically useless code is affected...)
 
 best regards
 Matthias