Re: [Wireshark-dev] Wireshark coding style help

[Bill Meier]

On 11/21/2013 8:05 PM, Guy Harris wrote:


On Nov 21, 2013, at 4:37 PM, Michael Lum
 wrote:


Can someone tell me why code like this:

i++;

would have been changed to this:

i += 1;

?


If the code in question is stepping through a packet, and "i" is
actually "offset" or some such variable holding the offset into the
packet, and other code is doing "offset += 2" or "offset += 4",
people might have used "offset += 1" to make the style more
consistent and to put the field length into all the incrementing
lines of code.



You may be referring to one  or more changes I made.  :)

As Guy suggested, for consistency I tend to change 'offset++' to 'offset 
+= 1'.


It does appear that I changed 'i++;' to 'i += 1;' in a few instances.
I must have been a bit over enthusiastic in those cases since there's no 
real reason for that change.


Bill

___
Sent via:Wireshark-dev mailing list 
Archives:http://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe

Re: [Wireshark-dev] Wireshark coding style help

[Michael Lum]

Thanks Bill.

I was concerned there was something I was missing.

I understand its a lot of work to go through other peoples varying styles of 
code.
Keep up the good work.

Thanks again.

--
Michael Lum (michael@starsolutions.com) | STAR SOLUTIONS | Principal 
Software Engineer
4600 Jacombs Road, Richmond BC, Canada V6V 3B1 | +1.604.303.2315
 

> -Original Message-
> From: wireshark-dev-boun...@wireshark.org 
> [mailto:wireshark-dev-boun...@wireshark.org] On Behalf Of Bill Meier
> Sent: November 22, 2013 11:34 AM
> To: Developer support list for Wireshark
> Subject: Re: [Wireshark-dev] Wireshark coding style help
> 
> On 11/21/2013 8:05 PM, Guy Harris wrote:
> >
> > On Nov 21, 2013, at 4:37 PM, Michael Lum 
> >  wrote:
> >
> >> Can someone tell me why code like this:
> >>
> >> i++;
> >>
> >> would have been changed to this:
> >>
> >> i += 1;
> >>
> >> ?
> >
> > If the code in question is stepping through a packet, and "i" is 
> > actually "offset" or some such variable holding the offset into the 
> > packet, and other code is doing "offset += 2" or "offset += 
> 4", people 
> > might have used "offset += 1" to make the style more 
> consistent and to 
> > put the field length into all the incrementing lines of code.
> >
> 
> You may be referring to one  or more changes I made.  :)
> 
> As Guy suggested, for consistency I tend to change 'offset++' 
> to 'offset 
> += 1'.
> 
> It does appear that I changed 'i++;' to 'i += 1;' in a few instances.
> I must have been a bit over enthusiastic in those cases since 
> there's no real reason for that change.
> 
> Bill
> 
> __
> _
> Sent via:Wireshark-dev mailing list 
> Archives:http://www.wireshark.org/lists/wireshark-dev
> Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
>  
> mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe
> 
___
Sent via:Wireshark-dev mailing list 
Archives:http://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe

Re: [Wireshark-dev] Status Cmake Win32 support

[Graham Bloice]

Back onto this.

Still attempting to build tshark via a Visual studio 2010 solution.

First failure, the regex stuff in FindGTK3.cmake around line 44 fails
if when I set WIRESHARK_BASE_DIR to E:\Wireshark for my build env.  It
breaks on the "\W" part.  Setting WIRESHARK_BASE_DIR to "E:/Wireshark"
fixes that, but this is odd for Windows folks.

Next, still failing on the command line passed to make-dissector-reg.py.
The command line in the epan.vcxproj file is good, but what ends up being
passed into the python script is decidedly odd.  I added debugging into the
script to write out all the files passed into the script and it seems that
every 8192 bytes of the command line parameter string, one is dropped ??

I guess we need to write the command line out to a temporary file somewhere
and then get the python script to pick that up.
___
Sent via:Wireshark-dev mailing list 
Archives:http://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe

[Wireshark-dev] How to replace proto_tree_add_text()

[Michael Lum]

I ran the checkAPIs.pl script against the files I have modified and it 
complains about a lot of proto_tree_add_text() calls.

I would like to fix them but after looking at the README.dissector document I'm 
left wondering if there is an easier way
than using proto_tree_add_item.

The places in the dissector where add_text() is used were for labelling parts 
of the protocol without having to create
filterable fields.

For example, in packet-ansi_a.c there are 457 useless add_text() calls vs 26 
add_() calls.

Do I have to add 457 items (approximately, some may duplicate) to 
hf_register_info ?

Basically, I want to just label a bunch of bits or octets, no subtree, not 
filterable.

Thanks for you help.


Michael Lum 
(michael@starsolutions.com) | STAR 
SOLUTIONS | Principal Software Engineer
4600 Jacombs Road, Richmond BC, Canada V6V 3B1 | +1.604.303.2315

___
Sent via:Wireshark-dev mailing list 
Archives:http://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe

Re: [Wireshark-dev] How to replace proto_tree_add_text()

[Guy Harris]

On Nov 22, 2013, at 2:10 PM, Michael Lum  wrote:

> I ran the checkAPIs.pl script against the files I have modified and it 
> complains about a lot of proto_tree_add_text() calls.
>  
> I would like to fix them but after looking at the README.dissector document 
> I'm left wondering if there is an easier way
> than using proto_tree_add_item.
>  
> The places in the dissector where add_text() is used were for labelling parts 
> of the protocol without having to create
> filterable fields.

A better term is "named fields", as a field's name can be used in places other 
than filter expressions - which, themselves, should perhaps be called 
"packet-testing expressions", as they're used for more than filtering; they can 
be used for coloring as well.

This means that:

> For example, in packet-ansi_a.c there are 457 useless add_text() calls vs 26 
> add_() calls.
>  
> Do I have to add 457 items (approximately, some may duplicate) to 
> hf_register_info ?
>  
> Basically, I want to just label a bunch of bits or octets, no subtree, not 
> filterable.

...a named field is more than "filterable"; it can be used to make a custom 
column, or can be used in TShark output with "-T fields", and possibly other 
places I've forgotten about.

That's why we encourage making named fields and using them - somebody might 
find a use for them, even if you don't have a use for them.
___
Sent via:Wireshark-dev mailing list 
Archives:http://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe

Re: [Wireshark-dev] How to replace proto_tree_add_text()

[mmann78]

I developed convert_proto_tree_add_text.pl (in tools dir) for just this reason. 
 See http://www.wireshark.org/lists/wireshark-dev/201307/msg00073.html for 
brief explanation.

If you're looking specifically at packet-ansi_a.c, I'm not sure how 
straightforward the conversion will be.  Without any knowledge of the protocol, 
I tried to look into using convert_proto_tree_add_text on it because 
packet-ansi_a.c is one of the worse offenders of proto_tree_add_text (over)use. 
 I didn't get far because I was confused by what I considered "extra logic" in 
trying to format fields and it just looks like the dissector needs an overhaul.

However if you're familiar with the protocol, I'll gladly assist you (you can 
email me directly) in trying to apply/use convert_proto_tree_add_text.pl to 
reduce the number of proto_tree_add_text calls.

Michael
 


-Original Message-
From: Guy Harris 
To: Developer support list for Wireshark 
Sent: Fri, Nov 22, 2013 5:39 pm
Subject: Re: [Wireshark-dev] How to replace proto_tree_add_text()



On Nov 22, 2013, at 2:10 PM, Michael Lum  wrote:

> I ran the checkAPIs.pl script against the files I have modified and it 
complains about a lot of proto_tree_add_text() calls.
>  
> I would like to fix them but after looking at the README.dissector document 
I'm left wondering if there is an easier way
> than using proto_tree_add_item.
>  
> The places in the dissector where add_text() is used were for labelling parts 
of the protocol without having to create
> filterable fields.

A better term is "named fields", as a field's name can be used in places other 
than filter expressions - which, themselves, should perhaps be called 
"packet-testing expressions", as they're used for more than filtering; they can 
be used for coloring as well.

This means that:

> For example, in packet-ansi_a.c there are 457 useless add_text() calls vs 26 
add_() calls.
>  
> Do I have to add 457 items (approximately, some may duplicate) to 
hf_register_info ?
>  
> Basically, I want to just label a bunch of bits or octets, no subtree, not 
filterable.

...a named field is more than "filterable"; it can be used to make a custom 
column, or can be used in TShark output with "-T fields", and possibly other 
places I've forgotten about.

That's why we encourage making named fields and using them - somebody might 
find 
a use for them, even if you don't have a use for them.
___
Sent via:Wireshark-dev mailing list 
Archives:http://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe

 

___
Sent via:Wireshark-dev mailing list 
Archives:http://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe