This discussion has been locked.

You can no longer post new replies to this discussion. If you have a question you can start a new discussion

Comparison not true

Hi Guys,

I'm writing my own TCP/IP Stack.
I've got following structure:

typedef struct  __attribute__ ((packed))
{
        uint16_t type                   : 16;
        uint16_t protocol               : 16;
        unsigned char hw_size           : 8;
        unsigned char prot_size         : 8;
        uint16_t op                     : 16;
        uint64_t srcMac                 : 48;
        uint32_t srcIP                  : 32;
        uint64_t dstMac                 : 48;
        uint32_t dstIP                  : 32;
}
ARP_Header;

If i want to compare the protocol value, the comparison isn't true, altough there is the right value in it.

if(rx_arp_header->protocol != ARP_PROT_Type_IP)
{
     DEBUG("Protocoll Error: %x\n", rx_arp_header->protocol);
     return;
}

ARP_PROT_Type_IP is defined by: #define ARP_PROT_Type_IP SWAP16(0x0800)

And SWAP16:

#define SWAP16(x)       ((x & 0x00FF) << 8) | \ 
                        ((x & 0xFF00) >> 8)

I have found a workaround: I have to cast the SWAP16 value to an integer. Why? The code size is increasing about 200 Bytes.

Can you help me?

br Robert

Parents

0 Stefan Hartwig over 16 years ago

Hi,
the problem is the SWAP16 macro.
The source code will be converted to

if(rx_arp_header->protocol != SWAP16(0x0800))

and then to

if(rx_arp_header->protocol != ((0x0800 & 0x00FF) << 8) | \ 
                              ((0x0800 & 0xFF00) >> 8)

As you may see, the SWAP16 must be in additional parentheses, otherwise, its always true

rx_arp_header->protocol != ((0x0800 & 0x00FF) << 8) // gives true or false
| ((0x0800 & 0xFF00) // always gives true

Change the define to:

#define SWAP16(x)       (((x & 0x00FF) << 8) | \ 
                        ((x & 0xFF00) >> 8))

Reply

0 Stefan Hartwig over 16 years ago

Hi,
the problem is the SWAP16 macro.
The source code will be converted to

if(rx_arp_header->protocol != SWAP16(0x0800))

and then to

if(rx_arp_header->protocol != ((0x0800 & 0x00FF) << 8) | \ 
                              ((0x0800 & 0xFF00) >> 8)

As you may see, the SWAP16 must be in additional parentheses, otherwise, its always true

rx_arp_header->protocol != ((0x0800 & 0x00FF) << 8) // gives true or false
| ((0x0800 & 0xFF00) // always gives true

Change the define to:

#define SWAP16(x)       (((x & 0x00FF) << 8) | \ 
                        ((x & 0xFF00) >> 8))

Children

No data