Ethereal-dev: Re: [Ethereal-dev] Van Jacobson dissector.

Note: This archive is from the project's previous web site, ethereal.com. This list is no longer active.

From: Irfan Khan <ikhan@xxxxxxxxxxxx>
Date: Wed, 19 Dec 2001 15:28:46 -0700
Thanks for the feed back. This is becoming more and more a learning experience :)
It ought to be helpful in my second dissector.

> /* IP and TCP header types */
> typedef struct {
> #if BYTE_ORDER == LITTLE_ENDIAN
>   guint8  ihl:4,
>           version:4;
> #else
>   guint8  version:4,
>           ihl:4;
> #endif

You should stay away from using bitfields like this
in portable code. ANSI says that only unsigned ints can
hold bitfields. GCC accepts the above, but not all
compilers do (e.g., IBM's C compiler).
Look at the code in packet-ip.c to see what we did
to make this portable.

The VJ code comes from Linux. The linux ip structure considers bit field order. I thought it was a good heurisitic to consider bitfield endian'ness to be the same as byte order - I may have been wrong. I have no problem with using the lo_nibble and hi_nibble approach.
Perhaps I can hold the world record of the fastest patch in the process :)

struct iphdr {
#if defined(__LITTLE_ENDIAN_BITFIELD)
        __u8    ihl:4,
                version:4;
#elif defined (__BIG_ENDIAN_BITFIELD)
        __u8    version:4,
                ihl:4;
#else
#error  "Please fix <asm/byteorder.h>"
#endif
        __u8    tos;
        __u16   tot_len;
        __u16   id;
        __u16   frag_off;
        __u8    ttl;
        __u8    protocol;
        __u16   check;
        __u32   saddr;
        __u32   daddr;
        /*The options start here. */
};



>   /* Copy header and form tvb */
>   hdr_len  = ((iphdr_type *)hdr_buf)->ihl * 4;
>   hdr_len += ((tcphdr_type *)(hdr_buf + hdr_len))->doff * 4;

We just had an issue with this type of struct-casting
in tcp_graph.c. We stay away from this. Because hdr_buf is
being allocated from a GMemChunk, it might be aligned
properly for what you're doing. Any opinions on this?


I couldn't find tcp_graph.c ??

>
>   /* Store the reconstructed header in frame data area */
>   buf_hdr = g_mem_chunk_alloc(vj_header_memchunk);

I don't see a g_mem_chunk_free() call for the buf_hdr's that
are allocated.


I use g_mem_chunk_destroy in the initialization function for vj for that.

Thanks,
Irfan

--gilbert