On Sep 2, 2008, at 2:04 PM, Maynard, Chris wrote:
> Jaap,
> I assume this is the line it's complaining about?:
>
> fcs_ok = (fcs == ieee802154_crc_tvb(tvb,
> tvb_reported_length(tvb)-IEEE802154_FCS_LEN));
>
> At first glance, there doesn't seem to be anything wrong with the
> comparison since fcs is a guint16 and that's exactly what
> crc16_ccitt_tvb_seed() returns; however, because the comparison is
> actually the following:
>
> fcs_ok = (fcs == (crc16_ccitt_tvb_seed(tvb,
> tvb_reported_length(tvb)-IEEE802154_FCS_LEN, IEEE802154_CRC_SEED) ^
> IEEE802154_CRC_XOROUT));
>
> I think the compiler is interpreting (blah ^
> IEEE802154_CRC_XOROUT) ...
> where IEEE802154_CRC_XOROUT is defined as 0xFFFF ... as the equivalent
> of (~blah) and that's where your warning is coming from.
At least as I read ANSI X3.159-1989 (i.e., C89), on the platforms on
which we run (where "int" and "unsigned int" are 32 bits and "short
int" and "unsigned short int" are 16 bits):
0xFFFF has type "int";
in an expression of the type
{unsigned short int} ^ {int}
the {unsigned short int} value is converted to int before being XORed
with the {int} value;
so
(crc16_ccitt_tvb_seed(tvb, tvb_reported_length(tvb)-
IEEE802154_FCS_LEN, IEEE802154_CRC_SEED) ^ IEEE802154_CRC_XOROUT)
first widens the result of crc16_ccitt_tvb_seed() (which is a uint16,
i.e. unsigned short int) to an int (which means no sign extension, as
a uint16 has no sign) and then XORs it with 0xFFFF. That's not ~-ing
it, as it only flips the lower 16 bits.
Then, again as I read C89:
in an expression of the type
{unsigned short int} == {int}
the {unsigned short int} value is converted to int before being
compared with the {int} value
so, in the expression
fcs == (blah blah blah)
"fcs" is widened to an int (again, no sign extension) and then
compared with the result.
Searching for information about this found
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=10226
where all of the code they show involves the ~ operator, not XORing.
The list of messages for that bug is a bit confusing (perhaps because
some messages include stuff from the previous messages), but I think
the first comment is
> When two unsigned shorts (16 bit) variables are compared with one
> being
> inverted the comparison will fail.
> The following code should pass, while it does generate a warning it
> should
> instead just work.
>
> unsigned short A = 0xDEAD;
> unsigned short B;
> B = ~A;
> if ( B == ~A) {
> printf("Pass\n");
> }
> else {
> printf("Fail\n");
> }
>
> It compares 0xFFFFDEAD == 0x0000DEAD which fails
followed by
> Apparently, you didn't understand the warning. The C standard mandates
> that "~A" will promote A to int first. So gcc is behaving correctly
> here. Any suggestions on how the wording of the message could be
> improved to be clearer?
and
> I'll leave the final decision to the language lawyers, but I don't
> think
> this is a bug in GCC. The ~ operator is subject to integer promotion,
> so with the implicit conversions the expression becomes:
> if ((int) B == ~((int) A))
> which is indeed false in the example above.
>
> In fact, I see the following warning when compiling with -Wall (GCC
> 3.3):
> warning: comparison of promoted ~unsigned with unsigned
>
> Perhaps "if (B == (unsigned short) ~A)" will behave as you expect.
followed by
> Actually I understood that the warning was tied to that error.
>
> I would suggest:
> warning : ~ operator caused promotion of unsigned short to int
>
> Interestingly: Sun CC passes and Microsoft Fails without warning.
In the case of
unsigned short A = 0xDEAD;
unsigned short B;
B = ~A;
if ( B == ~A) {
printf("Pass\n");
}
else {
printf("Fail\n");
}
on a 32-bit-int platform A (0xDEAD) gets promoted to
"int" (0x0000DEAD) and then ~ed (0xFFFF2152) and then assigned to a
"unsigned short" (0x2152). That "unsigned short" is then compared
with the un-shortened int, and the "unsigned short" gets widened to an
"int" first, so 0x2152 gets promoted to 0x00002152, and then compared
with ~A, which is 0xFFFF2152, and that comparison fails. In fact,
it'll always fail, because the comparison will be 0x0000XXXX with
0xFFFFXXXX.
However, in our case, I think the compiler is over-enthusiastically
turning XORing with 0xFFFF into ~ing - a simplified version would be
unsigned short A = 0xDEAD;
unsigned short B;
B = A ^ 0xFFFF;
if ( B == (A ^ 0xFFFF)) {
printf("Pass\n");
}
else {
printf("Fail\n");
}
in which case A (0xDEAD) gets promoted to "int" (0x0000DEAD) and then
XORed with 0xFFFF (0x00002152) and then assigned to an "unsigned
short" (0x2152), blah blah blah.
So I think this is a GCC bug; you might want to file a bug either
against Debian or against GCC and note that "{unsigned short} ^
0xFFFF" is not equivalent to "~{unsigned short}" in an "int longer
than short" environment (the first of those doesn't flip the upper N
bits of the promoted "unsigned short", the second of those does).
_______________________________________________
Wireshark-dev mailing list
[email protected]
https://wireshark.org/mailman/listinfo/wireshark-dev
