Hi, Sebastien, On Tue, 19 Dec 2006, Sebastien Tandel wrote:
> 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? No, you're absolutely right. I was called into a meeting and just posted the question. Keep it in there, it's correct. > > * 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? I rather would like a check against the actual size of the packet, to avoid going out of bounds at tvb access. > > * Use %u i.s.o. %d when printing unsigned values > done Great. > > * 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? :-/ True, and I don't like it, it's sloppy. So I'm gonna pound on them until they're gone. Thanks for taking them out. > > * 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 ... I mean the copyright stuff, like /* packet-PROTOABBREV.c * Routines for PROTONAME dissection * Copyright 200x, YOUR_NAME <YOUR_EMAIL_ADDRESS> * * $Id$ * * Wireshark - Network traffic analyzer * By Gerald Combs <[EMAIL PROTECTED]> * Copyright 1998 Gerald Combs * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License * as published by the Free Software Foundation; either version 2 * of the License, or (at your option) any later version. etc. Furhter question: did you fuzz test this dissector on some real life roofnet captures? Thanx, Jaap _______________________________________________ Wireshark-dev mailing list [email protected] http://www.wireshark.org/mailman/listinfo/wireshark-dev
