Wireshark · Wireshark-dev: Re: [Wireshark-dev] proto.h extension (unit strings)

[Pascal Quantin <pascal.quantin@xxxxxxxxx>]

Hi Michael,

2016-12-16 16:14 GMT+01:00 Michael Mann <mmann78@xxxxxxxxxxxx>:

I've finished my work adding unit string support and converting dissectors to use it (There are now over 300 examples of how to use this new feature).  I believe I was conservative in the conversion, so if anyone wants to check their favorite dissector(s) for additional use, go right ahead.  The most notable omission for conversion was ASN.1 files as I didn't investigate if I had to modify asn2wrs (nor was I interested in doing so).  packet-lpp.c, packet-lte-rrc.c, and packet-ulp.c in particular has several places where proto_item_append_text could be converted to BASE_UNIT_STRING in a hf_ field.

As I'm the author (culprit? ;) ), I will have a look at them. They will require manual modifcation as this is not something that could be deduced from the ASN.1 prose (and thus added to asn2wrs).

Cheers,
Pascal.

 

With the current functionality there are a few separate "units" for "full name" vs abbreviation.  Those that suggested they be combined are welcomed to do so.  I'm guessing 90%+ of the cases that use BASE_UNIT_STRING are using the "common" ones from unit_string.h, but do note that I did add a few specific ones in dissectors themselves (if the unit_name_string is going to be modified to include abbreviations directly).

 

There was some talk of scaling and my recommendation would be to have it work with "mx + b" with "x" as the field value, m = 1, b = 0 as default.

 

Also wanted to mention that I think Stig volunteered to add support on the Lua end (another area that I'm not familiar enough with to add support).  He can correct me, but I thought I'd put that out there to prevent duplication of work (in case others thought it was needed).

 

 

-----Original Message-----
From: Michael Mann <mmann78@xxxxxxxxxxxx>
To: wireshark-dev <wireshark-dev@xxxxxxxxxxxxx>
Sent: Mon, Dec 12, 2016 8:26 am
Subject: Re: [Wireshark-dev] proto.h extension (unit strings)

I'm not sure where the line between screaming "FEATURE CREAP!" and getting the patch "right" on the first try is.

 

A structure has been created to handle singular vs plural units and nothing else.  It can certainly be expanded for things like scaling, but let's take it one step at a time.  It's certainly early on in the 2.4 development cycle.

 

The "global" preference for (full) word vs abbreviation is an interesting one.  I'm all for as much "standardization" as possible, but also know that dissectors like their individual flexibility.  That's why for "seconds" I created 2 structures - one with full words (singular - "second", plural - "seconds") and one with abbreviation (singular - "s", plural - NULL (none)).  I can see a case for a few others, but I thought we'd end up with way more abbreviation structures than full word.

 

Once the original patch is submitted, I will take the masochistic task of search/replacing proto_tree_add_xxx_format_value and proto_item_append_text cases where their sole purpose is just adding a unit string and replacing it with this new unit string functionality.  This is where most of the real (time consuming) work is. It will have a positive impact on performance because it allows for lazier processing (and less printf-like handling) through proto_tree_add_item.  But that's also where people will probably notice the change with a "hey, the formatting is slightly different...".  One of the questions I had was how much do we standardize float values?  Are many of the proto_tree_add_[float|double]_format_value calls just because the dissector author didn't like the default precision of a float/double?  And in some (most?) cases they are probably justified.

 

 

-----Original Message-----
From: Michal Labedzki <michal.labedzki@xxxxxxxxx>
To: Developer support list for Wireshark <wireshark-dev@xxxxxxxxxxxxx>
Sent: Mon, Dec 12, 2016 3:47 am
Subject: Re: [Wireshark-dev] proto.h extension (unit strings)

Good idea.

I have similar problem with Bluetooth protocols. There is a lot of something like that:
"Latency = N * 0.625 ms (1 Baseband slot)"

So I want to display [ms] instead of [slot] (or maybe both? [new use case]). What should I do in that case (N * 0.625)?
1. use own BASE_CUSTOM (dissector files)
2. public-predefined base custom function
3. combine it with "BASE_UNIT" (unit_name_string_get_value in base custom function) or BASE_UNIT | BASE_CUSTOM?

There is a need to update README.dissector (etc.)

I think there should be abbrev-form of unit in "struct unit_name_string" (second, seconds, s) and provide user option to choose between full-form and abbrev. I am not sure, but I will probably use abbreviation. I am not sure if mixing abbrev/non-abbrev on dissector level is good idea.

On 12 December 2016 at 04:17, Michael Mann <mmann78@xxxxxxxxxxxx> wrote:

I thought this was a good idea, just took a while to get around to it:

https://code.wireshark.org/review/19211

 

 

-----Original Message-----
From: Guy Harris <guy@xxxxxxxxxxxx>
To: Developer support list for Wireshark <wireshark-dev@xxxxxxxxxxxxx>
Sent: Fri, May 8, 2015 3:09 pm
Subject: Re: [Wireshark-dev] proto.h extension


On May 8, 2015, at 7:06 AM, "John Dill" <John.Dill@xxxxxxxxxxxxxxxxx> wrote:

>> Message: 3
>> Date: Thu, 7 May 2015 11:29:22 -0700
>> From: Guy Harris <guy@xxxxxxxxxxxx>
>> To: Developer support list for Wireshark <wireshark-dev@xxxxxxxxxxxxx>
>> Subject: Re: [Wireshark-dev] proto.h extension

>> Message-ID: <CF2E7490-F023-49C2-8383-6C1A1394B4E7@xxxxxxxxxxxx>
>> Content-Type: text/plain; charset=iso-8859-1
>>
>> On May 7, 2015, at 8:13 AM, "John Dill" <John.Dill@xxxxxxxxxxxxxxxxx> wrote:
>>
>>> I have a couple of extensions that I created for the Wireshark baseline
>> that we're using (1.10.x). The diffs to proto.h and proto.c show the code
>> changes to add a couple of features that I've found useful, unit strings
>> and hiding the bits for bitmask header fields.
>>>
>>> http://codepad.org/KTGdEL1t
>>
>> You need more than
>>
>> /* Following constants have to be ORed with a base_display_e when dissector
>> * want to control aspects of the display string for a header_field_info */
>>
>> as a comment there - you need comments explaining what each of those flags actually *does*.
>
> Guy,
>
> Sorry it wasn't clear. Starting from this snippet...
>
> /*
> * BASE_UNIT_STRING - Append a unit string to the numeric value

That one's reasonable; I've thought of a similar option.

> *
> * When active, Wireshark will append a unit string declared as a
> * simple 'char *' for the 'strings' to the numeric value.

You might want to, instead, have it be a structure with a pair of strings, so that if the field has the value 1, you can print the singular rather than the plural, e.g.:

struct unit_names {
char *singular; /* name to use for 1 unit */
char *plural; /* name to use for < 1 or > 1 units */
};

struct unit_names feet {
"foot",
"feet"
};

{
&hf_MFD_State_Data_Slew_Elevation_Ch1,
{
"Slew_Elevation_Ch1",
"ndo.MFD_State_Data.Slew_Elevation_Ch1",
FT_FLOAT,
BASE_NONE | BASE_UNIT_STRING,
&feet,
0x0,
NULL,
HFILL
}
},

(For floating-point numbers, "1 unit" means "*exactly* 1 unit", i.e. an exact floating-point comparison with 1x2^0.)

We could either

1) require that both be non-null

or

2) assume that, if the plural is null, you can pluralize using the standard rules of English.

Does anybody have a preference there?

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

--

	Pozdrawiam / Best regards ------------------------------------------------------------------------------------------------------------- Michał Łabędzki, Software Engineer Tieto Corporation Product Development Services http://www.tieto.com / http://www.tieto.pl --- ASCII: Michal Labedzki location: Swobodna 1 Street, 50-088 Wrocław, Poland room: 5.01 (desk next to 5.08) --- Please note: The information contained in this message may be legally privileged and confidential and protected from disclosure. If the reader of this message is not the intended recipient, you are hereby notified that any unauthorised use, distribution or copying of this communication is strictly prohibited. If you have received this communication in error, please notify us immediately by replying to the message and deleting it from your computer. Thank You. --- Please consider the environment before printing this e-mail. --- Tieto Poland spółka z ograniczoną odpowiedzialnością z siedzibą w Szczecinie, ul. Malczewskiego 26. Zarejestrowana w Sądzie Rejonowym Szczecin-Centrum w Szczecinie, XIII Wydział Gospodarczy Krajowego Rejestru Sądowego pod numerem 0000124858. NIP: 8542085557. REGON: 812023656. Kapitał zakładowy: 4 271500 PLN

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

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