Re: preliminary code submission

[Brian Oleksa <oleksab () darkcornersoftware com>]

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 () wireshark org>
> Archives:    http://www.wireshark.org/lists/wireshark-dev
> Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
>              mailto:wireshark-dev-request () wireshark org?subject=unsubscribe
>

Attachment: packet-helen.c
Description:

___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <wireshark-dev () wireshark org>
Archives:    http://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
             mailto:wireshark-dev-request () wireshark org?subject=unsubscribe