2016-12-16 16:51 GMT+01:00 Michael Mann <[email protected]>:

> 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 <[email protected]>
> To: Developer support list for Wireshark <[email protected]>
> 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 <[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=unsubscr
>> ibe
>>
>
> ____________________________________________________________
> _______________
> 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

Reply via email to