Wireshark-dev: Re: [Wireshark-dev] Beginner article for custom dissector now on Code Project

From: Jaap Keuter <jaap.keuter@xxxxxxxxx>
Date: Mon, 02 Jul 2007 07:20:22 +0200

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@xxxxxxxxxxxxx
http://www.wireshark.org/mailman/listinfo/wireshark-dev