Hi Michael, 2016-12-16 16:14 GMT+01:00 Michael Mann <[email protected]>:
> 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 <[email protected]> > To: wireshark-dev <[email protected]> > 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 <[email protected]> > To: Developer support list for Wireshark <[email protected]> > 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 <[email protected]> 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 <[email protected]> >> To: Developer support list for Wireshark <[email protected]> >> 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" <[email protected]> >> wrote: >> >> >> Message: 3 >> >> Date: Thu, 7 May 2015 11:29:22 -0700 >> >> From: Guy Harris <[email protected]> >> >> To: Developer support list for Wireshark <[email protected]> >> >> Subject: Re: [Wireshark-dev] proto.h extension >> >> Message-ID: <[email protected]> >> >> Content-Type: text/plain; charset=iso-8859-1 >> >> >> >> On May 7, 2015, at 8:13 AM, "John Dill" <[email protected]> >> 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 <[email protected]> >> Archives: https://www.wireshark.org/lists/wireshark-dev >> Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev >> mailto:[email protected]?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 <[email protected]> > Archives: https://www.wireshark.org/lists/wireshark-dev > Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev > mailto:[email protected]?subject=unsubscribe > <[email protected]?subject=unsubscribe> > > ____________________________________________________________ > _______________ > Sent via: Wireshark-dev mailing list <[email protected]> > Archives: https://www.wireshark.org/lists/wireshark-dev > Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev > mailto:[email protected]?subject= > unsubscribe >
___________________________________________________________________________ Sent via: Wireshark-dev mailing list <[email protected]> Archives: https://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev mailto:[email protected]?subject=unsubscribe
