[prev in list] [next in list] [prev in thread] [next in thread] 

List:       wireshark-dev
Subject:    Re: [Wireshark-dev] unit_name_string for FT_STRING field types?
From:       Michael Mann via Wireshark-dev <wireshark-dev () wireshark ! org>
Date:       2017-09-11 21:57:18
Message-ID: 15e72f259e7-c07-18ce () webjasstg-vaa08 ! srv ! aolmail ! net
[Download RAW message or body]

[Attachment #2 (multipart/alternative)]


Are you suggesting "unit types" for "strings" or are you suggesting "unit types" for \
"string values that should really be considered integers or floats"?  
It certainly sounds like the latter and in which case I would suggest converting them \
in your dissector.  Numeric fields that are treated as numbers have more flexibility \
with comparison and math operations.

To me there isn't an argument here to have support for "true" strings and the \
proto_tree_add_string_format or proto_tree_add_string_format_value seems more \
appropriate.  
 
 
-----Original Message-----
From: John Dill <John.Dill@greenfieldeng.com>
To: wireshark-dev <wireshark-dev@wireshark.org>
Sent: Mon, Sep 11, 2017 4:56 pm
Subject: [Wireshark-dev] unit_name_string for FT_STRING field types?



I have a dissector for a protocol sending packets containing ASCII strings of a \
delimited format over a TCP stream.  
/AREA/NAME/FILLED/GREEN/1/2000/4000//
 
Sometimes the values are floating point, like
 
/ENV/-/-/1.0/90.0/100.0/-/-/-/5000.0//
 
I'm dissecting the format ok, but I can't use unit_name_string for these FT_STRING \
defined header fields.  
I see (in 2.4.1) that unit_name_string is disabled for FT_STRING \
(​tmp_fld_check_assert is not allowing hfinfo->strings), so I've been using \
proto_tree_add_string_format..., but wondering if there's potential to allow \
FT_STRING to use unit_name_string.  
One could classify the string contents as an integer or floating point value to pass \
to one of these functions:  
unit_name_string_get_value
unit_name_string_get_value64
unit_name_string_get_double
 
If the string is not a valid number, or out of range, I'm not sure what the proper \
error behavior should be.  Could be to ignore the 'strings' value, throw an assert, \
or malformed packet.  It's possible that a value is missing '-' but I wouldn't want \
it to mark the packet as bogus because of it.  
Mostly, it'd be easier putting the units in the header field definition instead of \
having a separate table of header field -> unit_name_string for these FT_STRING types \
and doing the checking/formatting myself.  
Does this idea seem compatible with proto.c?
 
Thanks,
John Dill
 

___________________________________________________________________________Sent via:  \
Wireshark-dev mailing list <wireshark-dev@wireshark.org>Archives:    \
https://www.wireshark.org/lists/wireshark-devUnsubscribe: \
https://www.wireshark.org/mailman/options/wireshark-dev             \
mailto:wireshark-dev-request@wireshark.org?subject=unsubscribe


[Attachment #5 (text/html)]

<font color='black' size='2' face='arial'>
<div style="color: black; font-family: arial,helvetica; font-size: 10pt;">

<div id="AOLMsgPart_2_b9ea522a-ebd9-43d2-b820-d07fb642b5ad">
<font color="black" face="arial" size="2">

<div>Are you suggesting "unit types" for "strings" or are you suggesting "unit types" \
for "string values that should really be considered integers or floats"?</div>



<div>&nbsp;</div>



<div>It certainly sounds like the latter and in which case I would suggest converting \
them in your dissector.&nbsp; Numeric fields that are treated as numbers have more \
flexibility with comparison and math operations.</div>

<div>&nbsp;</div>

<div>To me there isn't an argument here to have support for "true" strings and the \
proto_tree_add_string_format or proto_tree_add_string_format_value seems more \
appropriate.</div>



<div>&nbsp;</div>



<div>&nbsp;</div>



<div>&nbsp;</div>



<div style="color: black; font-family: arial,helvetica; font-size: \
10pt;">-----Original Message-----<br>

From: John Dill &lt;John.Dill@greenfieldeng.com&gt;<br>

To: wireshark-dev &lt;wireshark-dev@wireshark.org&gt;<br>

Sent: Mon, Sep 11, 2017 4:56 pm<br>

Subject: [Wireshark-dev] unit_name_string for FT_STRING field types?<br>

<br>




<div id="AOLMsgPart_1.2_81d61833-2873-4b44-8438-002f264bf377">
<style style="display: none;" \
type="text/css">#AOLMsgPart_1.2_81d61833-2873-4b44-8438-002f264bf377 td{color: \
black;}  .aolReplacedBody p { margin-top: 0px; margin-bottom: 0px; }</style>

<div class="aolReplacedBody" style="color: rgb(0, 0, 0); font-family: \
Calibri,Arial,Helvetica,sans-serif; font-size: 12pt; background-color: rgb(255, 255, \
255);" dir="ltr">


<div>I have a dissector for a protocol \
sending&nbsp;packets&nbsp;containing&nbsp;ASCII strings of a delimited format over a \
TCP stream.</div>




<div>&nbsp;</div>




<div>/AREA/NAME/FILLED/GREEN/1/2000/4000//</div>




<div>&nbsp;</div>




<div>Sometimes the values are floating point, like</div>




<div>&nbsp;</div>




<div>/ENV/-/-/1.0/90.0/100.0/-/-/-/5000.0//</div>




<div>&nbsp;</div>




<div>I'm dissecting the format ok, but I can't use unit_name_string for these \
FT_STRING defined header fields.</div>




<div>&nbsp;</div>




<div>I see (in 2.4.1) that unit_name_string is disabled for FT_STRING \
(​tmp_fld_check_assert is not allowing hfinfo-&gt;strings), so I've been using \
proto_tree_add_string_format..., but wondering if there's potential to allow \
FT_STRING to use unit_name_string.</div>




<div>&nbsp;</div>




<div>One could classify the string contents&nbsp;as an integer or floating point \
value to pass to one of these functions: </div>




<div>&nbsp;</div>




<div>unit_name_string_get_value</div>




<div>unit_name_string_get_value64</div>




<div>unit_name_string_get_double</div>




<div>&nbsp;</div>




<div>If the string is not a valid number, or out of range, I'm not sure what the \
proper error behavior should be.&nbsp; Could be to ignore the 'strings' value, throw \
an&nbsp;assert, or malformed packet.&nbsp; It's possible that a value is missing '-' \
but I wouldn't want it  to mark the packet as bogus because of it.</div>




<div>&nbsp;</div>




<div>Mostly, it'd be easier putting the units in the header field definition instead \
of having a separate table of header field -&gt; unit_name_string for these FT_STRING \
types and doing the checking/formatting myself.</div>




<div>&nbsp;</div>




<div>Does this idea seem compatible with proto.c?</div>




<div>&nbsp;</div>




<div>Thanks,</div>




<div>John Dill</div>




<div>&nbsp;</div>




</div>


</div>


___________________________________________________________________________
Sent via:    Wireshark-dev mailing list &lt;wireshark-<a \
                href="mailto:dev@wireshark.org" \
                target="_blank">dev@wireshark.org</a>&gt;
Archives:    <a href="https://www.wireshark.org/lists/wireshark-dev" \
                target="_blank">https://www.wireshark.org/lists/wireshark-dev</a>
Unsubscribe: <a href="https://www.wireshark.org/mailman/options/wireshark-dev" \
target="_blank">https://www.wireshark.org/mailman/options/wireshark-dev</a>  <a \
href="mailto:wireshark-dev-request@wireshark.org?subject=unsubscribe" \
target="_blank">mailto:wireshark-dev-request@wireshark.org?subject=unsubscribe</a></div>


</font>
</div>

</div>
</font>


[Attachment #6 (text/plain)]

___________________________________________________________________________
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

[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic