looks good.
nothing really apart from cosmetics
1,
+ tftp_info = conversation_get_proto_data(conversation, proto_tftp);
+ if (!tftp_info) {
+ tftp_info = se_alloc(sizeof(tftp_conv_info_t));
+ }
+ tftp_info->blocksize = blocksize;
+ conversation_add_proto_data(conversation,
proto_tftp, tftp_info);
It would look nicer imho to only add the conversation protocol data if
it did not already exist
something like
tftp_info=c_get_proto_data()
if(!ftp_info){
tftp_info=se_alloc();
c_add_proto_data()
}
tftp_info->blocksize=
2, why pass conversation to tftp_dissect_options() when you only want
it to extract the conversation data?
why not pass tftp_info as parameter instead?
if so you may also want to change so there will unconditionally be a
tftp_info assigned from early in dissect_tftp() just after it has
created the conversations :
i.e after these two lines :
+ DISSECTOR_ASSERT(conversation);
}
adding
tftp_info=c_get_proto_data()
if(!tftp_info){
tftp_info=se_alloc()
tftp_info-> initialize all fields
c_add_proto_data()
}
that would eliminate the need to get/add the proto data further down in the code
On 9/5/06, Joerg Mayer <jmayer@xxxxxxxxx> wrote:
As this is the first time I have coded something with conversations I'd
like to get some review before checking it in.
The attached patch will add handling of the blocksize option as of
rfc2348 (see bug 1092).
Ciao
Joerg
--
Joerg Mayer <jmayer@xxxxxxxxx>
We are stuck with technology when what we really want is just stuff that
works. Some say that should read Microsoft instead of technology.
