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