Hi Ken,

Did a quick review of your article. These are the point you could improve.

------------8<-----------
7.0 Your Dissector Code
You can use a text editor of your choice to open the 
packet-yourprotocol.c. Let's take it line by line:
#ifdef HAVE_CONFIG_H
# include "config.h"
#endif

#include <stdio.h>
#include <gmodule.h>
#include <glib.h>
#include <epan/packet.h>
#include <string.h>
------------8<-----------

Leave out the gmodule.h include. That is never needed and possibly 
confusing.

------------8<------------
void proto_reg_handoff_amin(void)
{
     static int initialized=FALSE;

     if (!initialized) {
------------8<------------

gboolean would be the correct type here.

------------8<------------
     if (proto_amin == -1) { /* execute protocol initialization only once
------------8<------------

This conditional is not needed and possibly confusing since the register 
function is called once.

------------8<------------
//We use tvb_memcpy to get our length value out (Host order)
         tvb_memcpy(tvb, (guint8 *)&length, offset, 4);
         proto_tree_add_uint(amin_header_tree, hf_amin_length, tvb, 
offset, 4, length);
------------8<------------

A poor way to get the length. If the protocol is really using length in 
host order on the wire it would be impossible to get big and little 
endian machines to talk.

------------8<------------
         if (check_col(pinfo->cinfo, COL_INFO)) {
             col_add_fstr(pinfo->cinfo, COL_INFO, "%d > %d Info Type:[%s]",
                 pinfo->srcport, pinfo->destport,
                 val_to_str(type, packettypenames, "Unknown Type:0x%02x"));
         }
------------8<------------

Never put the COL_INFO stuff under the "if (tree)" conditional because 
this would leave you info column empty when there's no filter and no 
colouring active.

Thanx,
Jaap

Ken Thompson wrote:
> I've recently published a beginner article on creating a custom
> dissector.  This article would not of been possible without the
> developers guide.
> 
> Note: The article is designed for the Win32 environment.
> 
> http://www.codeproject.com/useritems/custom_dissector.asp
> 
> Regards
> Ken
> _______________________________________________
> Wireshark-dev mailing list
> Wireshark-dev@wireshark.org
> http://www.wireshark.org/mailman/listinfo/wireshark-dev
> 

_______________________________________________
Wireshark-dev mailing list
Wireshark-dev@wireshark.org
http://www.wireshark.org/mailman/listinfo/wireshark-dev

Reply via email to