Wireshark · Wireshark-dev: Re: [Wireshark-dev] preliminary code submission

[Brian Oleksa <oleksab@xxxxxxxxxxxxxxxxxxxxxx>]

Jakub

Thanks for this feedback. It is always good to have an extra set of eyes :-)

I need to fix my IDE so it will format correctly. This is probably why 
you see a problem with the indentation.

I took care of the following...and attached the updated file:

- C++ comments fixed to use  /*.......*/
- duplicated month names is fixed (I declared this at the top)
- removed variable data_handle
- I removed the one call to check_col...

I left the other check_col in:

   if (check_col(pinfo->cinfo, COL_PROTOCOL)) {
       col_set_str(pinfo->cinfo, COL_PROTOCOL, PROTO_TAG_HELEN);
   }

This prints out the HELEN ....proto_tag_helen in the protocol name in 
the GUI.
If I remove this... it will just display UDP

Also... I am using

gfloat latitude;
latitude = tvb_get_ntohieee_float(tvb,offset);

Why do you think this variable is not being used..??

Thanks again..!!

This is great feedback

Brian







Jakub Zawadzki wrote:
> Hi,
>
> On Wed, Feb 03, 2010 at 11:44:40AM -0500, Brian Oleksa wrote:
>   
> > Again... any feedback is appreciated.
> >     
>
> - Inconsistent indentation (you use sometimes \t sometimes spaces)
> - C++ comments style.
> - using value_string struct is more proper way to map value with string than switch-es...
>
> - You have lot of unused variables, like: 
>   
> > gfloat latitude;
> > latitude = tvb_get_ntohieee_float(tvb,offset);
> >     
> or
>   
> > struct e_in6_addr address;
> > tvb_get_ipv6(tvb, offset, &address);
> >     
>
> - duplicated mon_names[]
>
> - check_col() is not needed.
> - data_handle is never used.
>
> Cheers.
> ___________________________________________________________________________
> Sent via:    Wireshark-dev mailing list <wireshark-dev@xxxxxxxxxxxxx>
> Archives:    http://www.wireshark.org/lists/wireshark-dev
> Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
>              mailto:wireshark-dev-request@xxxxxxxxxxxxx?subject=unsubscribe
>   
#ifdef HAVE_CONFIG_H
#include "config.h"
#endif

#include <stdio.h>
#include <glib.h>
#include <epan/packet.h>
#include <time.h>
#include <string.h>

#define PROTO_TAG_HELEN    "HELEN"

static int proto_helen = -1;

static dissector_handle_t helen_handle;

void dissect_helen(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree);

static int helen_port = 7636;
static const char *mon_names[12] = {"Jan","Feb","Mar","Apr","May","Jun","Jul","Aug","Sep","Oct","Nov","Dec"};

static gint hf_helen_magic = -1;
static gint hf_helen_checksum = -1;
static gint hf_helen_txTime = -1;

static gint hf_helen = -1;
static gint hf_helen_time = -1;
static gint hf_helen_ipv4 = -1;
static gint hf_helen_ipv6 = -1;
static gint hf_helen_nos = -1;
static gint hf_helen_flowname = -1;
static gint hf_helen_longitude = -1;
static gint hf_helen_latitude = -1;
static gint hf_helen_altitude = -1;
static gint hf_helen_bearing = -1;
static gint hf_helen_speed = -1;
static gint hf_helen_sequence_num = -1;
static gint hf_helen_gpsstatus = -1;
static gint hf_helen_source = -1;

static gint ett_helen = -1;
static gint ett_helen_time = -1;
static gint ett_helen_ipv4 = -1;
static gint ett_helen_ipv6 = -1;
static gint ett_helen_nos = -1;
static gint ett_helen_flowname = -1;
static gint ett_helen_longitude = -1;
static gint ett_helen_latitude = -1;
static gint ett_helen_altitude = -1;
static gint ett_helen_bearing = -1;
static gint ett_helen_speed = -1;
static gint ett_helen_sequence_num = -1;
static gint ett_helen_gpsstatus = -1;
static gint ett_helen_source = -1;


void proto_reg_handoff_helen(void) {
    static gboolean initialized = FALSE;

        if (!initialized) {
            helen_handle = create_dissector_handle(dissect_helen, proto_helen);
            dissector_add("udp.port", helen_port, helen_handle);
        }
    initialized = TRUE;
}

void proto_register_helen(void) {
    static hf_register_info hf[] = {
        { &hf_helen,
            { "Data", "helen.data", FT_NONE, BASE_NONE, NULL, 0x0,
                "HELEN PDU", HFILL}},
        { &hf_helen_magic,
            { "Magic Number", "helen.magicNumber", FT_UINT8, BASE_HEX, NULL, 0x0,
                "Magic Number", HFILL}},
        { &hf_helen_checksum,
           { "Checksum", "helen.checksum", FT_UINT64, BASE_DEC, NULL, 0x0,
                "Checksum", HFILL}},
        { &hf_helen_txTime,
            { "System Tx Time", "helen.SystemTxTime", FT_ABSOLUTE_TIME, BASE_NONE, NULL, 0x0,
                "System Tx Time", HFILL}},
        { &hf_helen_time,
	        { "Time", "helen.time", FT_ABSOLUTE_TIME, BASE_NONE, NULL, 0x0,
	            "Time", HFILL}},
	    { &hf_helen_ipv4,
			{ "IPv4", "helen.ipv4address", FT_IPv4, BASE_NONE, NULL, 0x0,
	            "IPv4", HFILL}},
	    { &hf_helen_ipv6,
			{ "IPv6", "helen.ipv6address", FT_IPv6, BASE_NONE, NULL, 0x0,
	            "IPv6", HFILL}},
        { &hf_helen_sequence_num,
			{ "Sequence Number", "helen.sequenceNumber", FT_UINT32, BASE_DEC, NULL, 0x0,
                "Sequence Number", HFILL}},
        { &hf_helen_source,
			{ "Source", "helen.source", FT_UINT32, BASE_DEC, NULL, 0x0,
                "Source", HFILL}},
        { &hf_helen_nos,
			{ "Number of Satellites", "helen.numberOfSatellites", FT_UINT8, BASE_DEC, NULL, 0x0,
			"Number of satellites", HFILL}},
		{ &hf_helen_flowname,
			{ "Flowname", "helen.flowname", FT_STRING, BASE_NONE, NULL, 0x0,
			"Flowname", HFILL}},
		{ &hf_helen_gpsstatus,
			{ "GPS Status", "helen.gpsStatus", FT_STRING, BASE_NONE, NULL, 0x0,
			"GPS Status", HFILL}},
		{ &hf_helen_longitude,
			{ "Longitude", "helen.longitude", FT_FLOAT, BASE_DEC, NULL, 0x0,
			"Longitude", HFILL}},
		{ &hf_helen_latitude,
			{ "Latitude", "helen.latitude", FT_FLOAT, BASE_DEC, NULL, 0x0,
			"Latitude", HFILL}},
		{ &hf_helen_altitude,
			{ "Altitude", "helen.altitude", FT_FLOAT, BASE_DEC, NULL, 0x0,
			"Altitude", HFILL}},
		{ &hf_helen_bearing,
			{ "Bearing", "helen.bearing", FT_FLOAT, BASE_DEC, NULL, 0x0,
			"Bearing", HFILL}},
		{ &hf_helen_speed,
			{ "Speed", "helen.speed", FT_FLOAT, BASE_DEC, NULL, 0x0,
			"Speed", HFILL}},
    };
    static gint * ett[] = {
        &ett_helen,
        &ett_helen_time,
        &ett_helen_ipv4,
        &ett_helen_ipv6,
        &ett_helen_nos,
        &ett_helen_flowname,
        &ett_helen_gpsstatus,
        &ett_helen_sequence_num,
        &ett_helen_source,
        &ett_helen_longitude,
        &ett_helen_latitude,
        &ett_helen_altitude,
        &ett_helen_bearing,
        &ett_helen_speed,
    };

    proto_helen = proto_register_protocol("HELEN", "HELEN", "helen");
    proto_register_field_array(proto_helen, hf, array_length(hf));
    proto_register_subtree_array(ett, array_length(ett));
    register_dissector("helen", dissect_helen, proto_helen);
}

void dissect_helen(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree) {

    proto_item *helen_item = NULL;
    proto_item *helen_sub_item = NULL;
    proto_tree *helen_tree = NULL;
    proto_tree *helen_header_tree = NULL;

    if (check_col(pinfo->cinfo, COL_PROTOCOL)) {
        col_set_str(pinfo->cinfo, COL_PROTOCOL, PROTO_TAG_HELEN);
    }

    if (tree) {
        guint32 offset = 0;
        guint32 orig_offset = 18;
        nstime_t t;
		guint64 msecs_since_the_epoch;
		struct tm *tmp;
        helen_item = proto_tree_add_item(tree, proto_helen, tvb, 0, -1, FALSE);
        helen_tree = proto_item_add_subtree(helen_item, ett_helen);
        helen_header_tree = proto_item_add_subtree(helen_item, ett_helen);
        helen_sub_item = proto_tree_add_item(helen_tree, hf_helen_magic, tvb, offset, 2, FALSE);
        offset += 2;
		helen_sub_item = proto_tree_add_item(helen_tree, hf_helen_checksum, tvb, offset, 8, FALSE);
		offset += 8;
		msecs_since_the_epoch = tvb_get_ntoh64(tvb, offset);
		t.secs = msecs_since_the_epoch/1000;
		t.nsecs = (msecs_since_the_epoch%1000)*1000000;	/* milliseconds to nanoseconds */
		tmp = gmtime(&t.secs);
        proto_tree_add_time_format(helen_tree, hf_helen_txTime, tvb, offset, 8, &t,
            "Date: %s %2d, %d %02d:%02d:%02d UTC",mon_names[tmp->tm_mon],tmp->tm_mday,
			tmp->tm_year + 1900,tmp->tm_hour,tmp->tm_min,tmp->tm_sec,(long)t.nsecs);

        helen_header_tree = proto_item_add_subtree(helen_sub_item, ett_helen);
        {
            #define MAX_BUFFER 100
			char *buf = (char*)ep_alloc(MAX_BUFFER);
            char * packet_name = "";
            proto_tree *helen_sub_tree = NULL;
            offset = 18;

            for (;;) {
                guint16 code;
                guint16 numBytes = 0;
                guint unknownPacket = 0;
                guint codeOffset;
                offset = orig_offset;
		    	code = tvb_get_ntohs(tvb, offset);
                codeOffset = offset;
                offset += 2;
                numBytes = tvb_get_ntohs(tvb, offset);
                offset += 2;

                switch (code) {
                    case 0: packet_name = "End of Packet";
                        break;
                    case 1: packet_name = "GPS Extension";
                        break;
                    case 2: packet_name = "Flow Extension";
                        break;
                    case 3: packet_name = "Host Extension";
                        break;
                    default: packet_name = "Unknown code";
                        unknownPacket = 1;
                        break;
                }

                g_snprintf(buf, MAX_BUFFER, "%s", packet_name);

                if (unknownPacket) {
                    g_snprintf(buf, MAX_BUFFER, "Unknown packet: %d", code);
                }
                helen_item = proto_tree_add_text(tree, tvb, codeOffset, 2, "%s", buf);
                helen_sub_tree = proto_item_add_subtree(helen_item, ett_helen);

                if (code == 0) {
                    break;
                }

                /* GPS: */
                if (code == 1) {
                    guint8 fieldsAvail;
                    fieldsAvail = tvb_get_guint8(tvb, offset);
                    offset += 1;

                    /* Status: */
                    if ((fieldsAvail & 1) != 0) {
                        guint8 status;
                        char * statusStr = "";
                        status = tvb_get_guint8(tvb,offset);
                        if (status == 0) {
                            statusStr = "Good";
                        } else if (status == 1) {
                            statusStr = "No Fix";
                        } else if (status == 2) {
                            statusStr = "Bad GPS Read";
                        }
                        proto_tree_add_string_format(helen_sub_tree, hf_helen_gpsstatus, tvb, offset, 1, statusStr,
                                "GPS Status: %s", statusStr);
                        offset += 1;
                    }

                    /* Time: */
                    if ((fieldsAvail & 2) != 0) {
						nstime_t t;
						guint64 msecs_since_the_epoch;
						struct tm *tmp;
					    msecs_since_the_epoch = tvb_get_ntoh64(tvb, offset);
						t.secs = msecs_since_the_epoch/1000;
						t.nsecs = (msecs_since_the_epoch%1000)*1000000;	/* milliseconds to nanoseconds */
						tmp = gmtime(&t.secs);
                        proto_tree_add_time_format(helen_sub_tree, hf_helen_time, tvb, offset, 8, &t,
                                "Date: %s %2d, %d %02d:%02d:%02d UTC",mon_names[tmp->tm_mon],tmp->tm_mday,
								tmp->tm_year + 1900,tmp->tm_hour,tmp->tm_min,tmp->tm_sec,(long)t.nsecs);
                        offset += 8;
                    }

                    /* Longitude: */
                    if ((fieldsAvail & 4) != 0) {
                        gfloat longitude;
                        longitude = tvb_get_ntohieee_float(tvb,offset);
                        proto_tree_add_item(helen_sub_tree, hf_helen_longitude, tvb, offset, 4, FALSE);
                        offset += 4;
                    }

                    /* Latitude: */
                    if ((fieldsAvail & 8) != 0) {
                        gfloat latitude;
                        latitude = tvb_get_ntohieee_float(tvb,offset);
                        proto_tree_add_item(helen_sub_tree, hf_helen_latitude, tvb, offset, 4, FALSE);
                        offset += 4;
                    }

                    /* Altitude: */
                    if ((fieldsAvail & 16) != 0) {
                        gfloat altitude;
                        altitude = tvb_get_ntohieee_float(tvb,offset);
                        proto_tree_add_item(helen_sub_tree, hf_helen_altitude, tvb, offset, 4, FALSE);
                        offset += 4;
                    }

                    /* Bearing: */
                    if ((fieldsAvail & 32) != 0) {
                        gfloat bearing;
                        bearing = tvb_get_ntohieee_float(tvb,offset);
                        proto_tree_add_item(helen_sub_tree, hf_helen_bearing, tvb, offset, 4, FALSE);
                        offset += 4;
                    }

                    /* Speed: */
                    if ((fieldsAvail & 64) != 0) {
                        gfloat speed;
                        speed = tvb_get_ntohieee_float(tvb,offset);;
                        if (speed != 0.0) {
							proto_tree_add_item(helen_sub_tree, hf_helen_speed, tvb, offset, 4, FALSE);
                        }
                        offset += 4;
                    }

                    /* Number of Satellites: */
                    if ((fieldsAvail & 128) != 0) {
                        guint8 nos;
                        nos = tvb_get_guint8(tvb,offset);
                        proto_tree_add_item(helen_sub_tree, hf_helen_nos, tvb, offset, 1, FALSE);
                        offset += 1;
                    }
                }

                /* FLOW: */
                if (code == 2) {
                    guint32 seq;
                    guint32 src;

                    proto_tree_add_item(helen_sub_tree, hf_helen_flowname, tvb, offset, 8, FALSE);
                    offset += 8;

                    /* Sequence number: */
                    seq = tvb_get_ntohl(tvb,offset);
        			proto_tree_add_item(helen_sub_tree, hf_helen_sequence_num, tvb, offset, 4, FALSE);
                    offset += 4;

                    if (numBytes == 16) {
					   /* Source: */
					   src = tvb_get_ntohl(tvb,offset);
					   proto_tree_add_item(helen_sub_tree, hf_helen_source, tvb, offset, 4, FALSE);
					   offset += 4;
                    }
                }

                /* HOST: */
                if (code == 3) {
                    /* Size: */
                    guint8 size;
                    size = tvb_get_guint8(tvb, offset);
                    offset += 1;

                    if (size == 4) {
                        guint32 addr;
						addr = tvb_get_ipv4(tvb,offset);
                        proto_tree_add_item(helen_sub_tree, hf_helen_ipv4, tvb, offset, 4, FALSE);
                        offset += 4;
                    } else
                    {
						struct e_in6_addr address;
						tvb_get_ipv6(tvb, offset, &address);
						proto_tree_add_item(helen_sub_tree, hf_helen_ipv6, tvb, offset, 16, FALSE);
						offset += 16;
					}
			    }
                orig_offset += numBytes + 4;
            }
        }
    }
}