Wireshark-dev: Re: [Wireshark-dev] Wireshark-dev Digest, Vol 207, Issue 3

From: John Dill <John.Dill@xxxxxxxxxxxxxxxxx>
Date: Tue, 15 Aug 2023 13:03:39 +0000

Here is a sample from my implementation.


proto.c snippets


static const char *hf_try_val_to_str(guint32 value, const header_field_info *hfinfo);
static const char *hf_try_val_to_str_idx(guint32 value, const header_field_info *hfinfo, gint *idx);
static const char *hf_try_val64_to_str(guint64 value, const header_field_info *hfinfo);
static const char *hf_try_val64_to_str_idx(guint64 value, const header_field_info *hfinfo, gint *idx);
...
static const char *
hf_try_val_to_str_idx(guint32 value, const header_field_info *hfinfo, gint *idx)
{
if (hfinfo->display & BASE_RANGE_STRING)
return try_rval_to_str_idx(value, (const range_string *) hfinfo->strings, idx);

if (hfinfo->display & BASE_EXT_STRING) {
if (hfinfo->display & BASE_VAL64_STRING)
return try_val64_to_str_idx_ext(value, (val64_string_ext *) hfinfo->strings, idx);
else
return try_val_to_str_idx_ext(value, (value_string_ext *) hfinfo->strings, idx);
}

if (hfinfo->display & BASE_VAL64_STRING)
return try_val64_to_str_idx(value, (const val64_string *) hfinfo->strings, idx);

if (hfinfo->display & BASE_UNIT_STRING) {
/* If this is called with a unit name string, something isn't right. */
REPORT_DISSECTOR_BUG("field %s strings is a unit name string", hfinfo->abbrev);
}

return try_val_to_str_idx(value, (const value_string *) hfinfo->strings, idx);
}
...

static const char *
hf_try_val64_to_str_idx(guint64 value, const header_field_info *hfinfo, gint *idx)
{
if (hfinfo->display & BASE_VAL64_STRING) {
if (hfinfo->display & BASE_EXT_STRING)
return try_val64_to_str_idx_ext(value, (val64_string_ext *) hfinfo->strings, idx);
else
return try_val64_to_str_idx(value, (const val64_string *) hfinfo->strings, idx);
}

if (hfinfo->display & BASE_RANGE_STRING)
return try_rval64_to_str_idx(value, (const range_string *) hfinfo->strings, idx);

if (hfinfo->display & BASE_UNIT_STRING) {
/* If this is called with a unit name string, something isn't right. */
REPORT_DISSECTOR_BUG("field %s strings is a unit name string", hfinfo->abbrev);
}

/* If this is reached somebody registered a 64-bit field with a 32-bit
* value-string, which isn't right. */
REPORT_DISSECTOR_BUG("field %s is a 64-bit field with a 32-bit value_string",
    hfinfo->abbrev);

/* This is necessary to squelch MSVC errors; is there
   any way to tell it that DISSECTOR_ASSERT_NOT_REACHED()
   never returns? */
return NULL;
}
...
/* Fills data for bitfield ints with val_strings */
static void
fill_label_bitfield(field_info *fi, gchar *label_str, gboolean is_signed)
{
char       *p;
int         bitfield_byte_length, bitwidth;
guint32     unshifted_value;
guint32     value;

char        buf[32];
const char *out;
gint        idx;

header_field_info *hfinfo = fi->hfinfo;

/* Figure out the bit width */
bitwidth = hfinfo_container_bitwidth(hfinfo);

/* Un-shift bits */
if (is_signed)
value = fvalue_get_sinteger(&fi->value);
else
value = fvalue_get_uinteger(&fi->value);

unshifted_value = value;
if (hfinfo->bitmask) {
unshifted_value <<= hfinfo_bitshift(hfinfo);
}

/* Create the bitfield first */
p = decode_bitfield_value(label_str, unshifted_value, hfinfo->bitmask, bitwidth);
bitfield_byte_length = (int) (p - label_str);

/* Fill in the textual info using stored (shifted) value */
if (hfinfo->display == BASE_CUSTOM) {
gchar tmp[ITEM_LABEL_LENGTH];
const custom_fmt_func_t fmtfunc = (const custom_fmt_func_t)hfinfo->strings;

DISSECTOR_ASSERT(fmtfunc);
fmtfunc(tmp, value);
label_fill(label_str, bitfield_byte_length, hfinfo, tmp);
}
else if (hfinfo->strings) {
hf_try_val_to_str_idx(value, hfinfo, &idx);
const char* val_str = 
hf_try_val_to_str_const(
(hfinfo->display & BASE_DEFAULT_VALS && idx == -1) ? -1 : value, hfinfo, "Unknown");

out = hfinfo_number_vals_format(hfinfo, buf, value);
if (out == NULL) /* BASE_NONE so don't put integer in descr */
label_fill(label_str, bitfield_byte_length, hfinfo, val_str);
else
label_fill_descr(label_str, bitfield_byte_length, hfinfo, val_str, out);
}
else {
out = hfinfo_number_value_format(hfinfo, buf, value);

label_fill(label_str, bitfield_byte_length, hfinfo, out);
}
}

static void
fill_label_bitfield64(field_info *fi, gchar *label_str, gboolean is_signed)
{
char       *p;
int         bitfield_byte_length, bitwidth;
guint64     unshifted_value;
guint64     value;

char        buf[48];
const char *out;
gint        idx;

header_field_info *hfinfo = fi->hfinfo;

/* Figure out the bit width */
bitwidth = hfinfo_container_bitwidth(hfinfo);

/* Un-shift bits */
if (is_signed)
value = fvalue_get_sinteger64(&fi->value);
else
value = fvalue_get_uinteger64(&fi->value);

unshifted_value = value;
if (hfinfo->bitmask) {
unshifted_value <<= hfinfo_bitshift(hfinfo);
}

/* Create the bitfield first */
p = decode_bitfield_value(label_str, unshifted_value, hfinfo->bitmask, bitwidth);
bitfield_byte_length = (int) (p - label_str);

/* Fill in the textual info using stored (shifted) value */
if (hfinfo->display == BASE_CUSTOM) {
gchar tmp[ITEM_LABEL_LENGTH];
const custom_fmt_func_64_t fmtfunc64 = (const custom_fmt_func_64_t)hfinfo->strings;

DISSECTOR_ASSERT(fmtfunc64);
fmtfunc64(tmp, value);
label_fill(label_str, bitfield_byte_length, hfinfo, tmp);
}
else if (hfinfo->strings) {
hf_try_val64_to_str_idx(value, hfinfo, &idx);
const char* val_str = 
hf_try_val64_to_str_const(
(hfinfo->display & BASE_DEFAULT_VALS && idx == -1) ? -1 : value, hfinfo, "Unknown");

out = hfinfo_number_vals_format64(hfinfo, buf, value);
if (out == NULL) /* BASE_NONE so don't put integer in descr */
label_fill(label_str, bitfield_byte_length, hfinfo, val_str);
else
label_fill_descr(label_str, bitfield_byte_length, hfinfo, val_str, out);
}
else {
out = hfinfo_number_value_format64(hfinfo, buf, value);

label_fill(label_str, bitfield_byte_length, hfinfo, out);
}
}

That should get someone in the ballpark if there's interest in pursuing this feature further.  I'm sure there's more stuff that could be tweaked, but this works for me at the moment.

Thanks,
John D.

>Message: 1
>Date: Mon, 14 Aug 2023 15:22:01 +0000
>From: John Dill <John.Dill@xxxxxxxxxxxxxxxxx>
>To: "wireshark-dev@xxxxxxxxxxxxx" <wireshark-dev@xxxxxxxxxxxxx>
>Subject: [Wireshark-dev] add a BASE_DEFAULT_VALS
>Message-ID: <7842e9e621b04ca6a8749b5286f2d91a@xxxxxxxxxxxxxxxxx>
>Content-Type: text/plain; charset="iso-8859-1"
>
>I've recently been doing a lot of enums that have multiple illegal values, and

>the illegal value shouldn't be displayed as "Unknown" as it's hard coded in

>proto.c (in 3.6.x).
>
>Any chance you could go for an attribute to signal that -1 can be used as the

>name of the fall-through text if defined?
>
>Looks like it would be mostly proto that gets updated.
>
>#define BASE_DEFAULT_VALS         0x00040000  /**< field will use -1 value instead

>of "Unknown" if value_string match is not found */
>
>static const value_string Port_Test_Control_enum_vals[] =
>{
>  { -1, "Disable Manual Port Test Control" },
>  { 0x5A, "Enable Manual Port Test Control" },
>  { 0, NULL }
>};
>
>Here's a hf definition
>

>    {
>      &hf_Port_Test_Control_Enable,
>      {
>        "Port Test Control Enable",
>        "mil_1553.Port_Test_Control_Enable",
>        FT_UINT16,
>        BASE_HEX | BASE_DEFAULT_VALS,
>        VALS(Port_Test_Control_Enable_enum_vals),
>        0x00FF,
>        "This byte is always set to 0x00 when not in debug mode.  Only special debug functions may set this byte to 0x5A.",
>        HFILL
>      }
>    },
>

>Most of the time, my enum fields don't use -1 as a valid value, so UINT32(-1)

>should be pretty feasible.
>
>There are _idx functions in value_string, so may be able to rig something there.

>  I have made my own version of it but I'm sure someone more experienced can

>do it the "right" way in the code base.
>
>Just an idea.  I'm still on 3.6 so I haven't checked if a similar feature is in 4.0.
>
>Thanks,
>John D.


From: Wireshark-dev <wireshark-dev-bounces@xxxxxxxxxxxxx> on behalf of wireshark-dev-request@xxxxxxxxxxxxx <wireshark-dev-request@xxxxxxxxxxxxx>
Sent: Tuesday, August 15, 2023 8:00:01 AM
To: wireshark-dev@xxxxxxxxxxxxx
Subject: Wireshark-dev Digest, Vol 207, Issue 3
 
Send Wireshark-dev mailing list submissions to
        wireshark-dev@xxxxxxxxxxxxx

To subscribe or unsubscribe via the World Wide Web, visit
        https://www.wireshark.org/mailman/listinfo/wireshark-dev
or, via email, send a message with subject or body 'help' to
        wireshark-dev-request@xxxxxxxxxxxxx

You can reach the person managing the list at
        wireshark-dev-owner@xxxxxxxxxxxxx

When replying, please edit your Subject line so it is more specific
than "Re: Contents of Wireshark-dev digest..."


Today's Topics:

   1. add a BASE_DEFAULT_VALS (John Dill)


----------------------------------------------------------------------

Message: 1
Date: Mon, 14 Aug 2023 15:22:01 +0000
From: John Dill <John.Dill@xxxxxxxxxxxxxxxxx>
To: "wireshark-dev@xxxxxxxxxxxxx" <wireshark-dev@xxxxxxxxxxxxx>
Subject: [Wireshark-dev] add a BASE_DEFAULT_VALS
Message-ID: <7842e9e621b04ca6a8749b5286f2d91a@xxxxxxxxxxxxxxxxx>
Content-Type: text/plain; charset="iso-8859-1"

I've recently been doing a lot of enums that have multiple illegal values, and the illegal value shouldn't be displayed as "Unknown" as it's hard coded in proto.c (in 3.6.x).


Any chance you could go for an attribute to signal that -1 can be used as the name of the fall-through text if defined?


Looks like it would be mostly proto that gets updated.


#define BASE_DEFAULT_VALS         0x00040000  /**< field will use -1 value instead of "Unknown" if value_string match is not found */

static const value_string Port_Test_Control_enum_vals[] =
{
  { -1, "Disable Manual Port Test Control" },
  { 0x5A, "Enable Manual Port Test Control" },
  { 0, NULL }
};


Here's a hf definition


    {
      &hf_Port_Test_Control_Enable,
      {
        "Port Test Control Enable",
        "mil_1553.Port_Test_Control_Enable",
        FT_UINT16,
        BASE_HEX | BASE_DEFAULT_VALS,
        VALS(Port_Test_Control_Enable_enum_vals),
        0x00FF,
        "This byte is always set to 0x00 when not in debug mode.  Only special debug functions may set this byte to 0x5A.",
        HFILL
      }
    },

Most of the time, my enum fields don't use -1 as a valid value, so UINT32(-1) should be pretty feasible.


There are _idx functions in value_string, so may be able to rig something there.  I have made my own version of it but I'm sure someone more experienced can do it the "right" way in the code base.


Just an idea.  I'm still on 3.6 so I haven't checked if a similar feature is in 4.0.


Thanks,

John D.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://www.wireshark.org/lists/wireshark-dev/attachments/20230814/6339cb57/attachment.htm>

------------------------------

Subject: Digest Footer

_______________________________________________
Wireshark-dev mailing list
Wireshark-dev@xxxxxxxxxxxxx
https://www.wireshark.org/mailman/listinfo/wireshark-dev


------------------------------

End of Wireshark-dev Digest, Vol 207, Issue 3
*********************************************