Wireshark-dev: Re: [Wireshark-dev] [PATCH] New Dissector : Roofnet
From: Sebastien Tandel <sebastien@xxxxxxxxx>
Date: Tue, 19 Dec 2006 18:02:18 +0100
Hi Jaap,


> * I'm not sure about using get_hostname here. Shouldn't that be handled
> through Wireshark services.
>   
What do you mean? get_hostname is already part of the wireshark API
(addr_resolv) and is used in many dissectors. Should I use another function?

> * The loop in dissect_roofnet should check that it doesn't spin out of
> control when an incorrectly large value is read.
>   
The roofnet length is restricted to 400 bytes (maybe 200 in a near
future). I then control whetherr the length of the announced number of
links is greater than this max length (400). If it's the case I print an
error in the tree, add an expert info value and stop the dissection of
the packet. Is it sufficient?

> * Use %u i.s.o. %d when printing unsigned values
done
> * Don't put a comma after the last entry in a structure array initializer
>   
I'm ok with that (and it's done) but do you know there are around 4100
structures initialized like that only in the dissectors part? :-/
> * Use the 'standard' file header as found in the README.developer
>   
Did you mean stdio, stdlib ? If not, give me a hint 'cause I don't see ...


Regards,
Sebastien Tandel