Wireshark mailing list archives

Previous By Date Next
Previous By Thread Next

Re: proto.h extension (unit strings)

From: Pascal Quantin <pascal.quantin () gmail com>
Date: Fri, 16 Dec 2016 16:54:19 +0100
2016-12-16 16:51 GMT+01:00 Michael Mann <mmann78 () netscape net>:


If I remember correctly, I thought a lot of them required a "manual"
function (from .cnf file) whose sole purpose was to do the
proto_item_append_text call.  I didn't know if because its in the .cnf file
if that means the hf_ field would be part of the template file or if you
removed the "manual" function, if that's where the asn2wrs engine would
come in.



In fact I will have to invoke another magic (replacing the function by an
override of the hf variable parameters) in the .cnf file ;)





-----Original Message-----
From: Pascal Quantin <pascal.quantin () gmail com>
To: Developer support list for Wireshark <wireshark-dev () wireshark org>
Sent: Fri, Dec 16, 2016 10:18 am
Subject: Re: [Wireshark-dev] proto.h extension (unit strings)

Hi Michael,

2016-12-16 16:14 GMT+01:00 Michael Mann <mmann78 () netscape net>:


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 () netscape net>
To: wireshark-dev <wireshark-dev () wireshark org>
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 () tieto com>
To: Developer support list for Wireshark <wireshark-dev () wireshark org>
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 () netscape net> 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 () alum mit edu>
To: Developer support list for Wireshark <wireshark-dev () wireshark org>
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 () greenfieldeng com>
wrote:


Message: 3
Date: Thu, 7 May 2015 11:29:22 -0700
From: Guy Harris <guy () alum mit edu>
To: Developer support list for Wireshark <wireshark-dev () wireshark org



Subject: Re: [Wireshark-dev] proto.h extension
Message-ID: <CF2E7490-F023-49C2-8383-6C1A1394B4E7 () alum mit edu>
Content-Type: text/plain; charset=iso-8859-1

On May 7, 2015, at 8:13 AM, "John Dill" <John.Dill () greenfieldeng com>

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





--

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 <(854)%20208-5557>.
REGON: 812023656. Kapitał zakładowy: 4 271500 PLN
____________________________________________________________
_______________
Sent via: Wireshark-dev mailing list <wireshark-dev () wireshark org>
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
<wireshark-dev-request () wireshark org?subject=unsubscribe>

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



____________________________________________________________
_______________
Sent via: Wireshark-dev mailing list <wireshark-dev () wireshark org>
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
<wireshark-dev-request () wireshark org?subject=unsubscribe>

____________________________________________________________
_______________
Sent via:    Wireshark-dev mailing list <wireshark-dev () wireshark org>
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 () wireshark org>
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
Previous By Date Next
Previous By Thread Next

Current thread: