Skip to content

Conversation

@Boolean263
Copy link
Contributor

@Boolean263 Boolean263 commented Oct 28, 2025

Some minor tweaks to the coding style of the Wireshark dissector to bring it closer to the standard practices used in the Wireshark project itself. All changes are compatible with Wireshark 4.6 and should be with 4.4 as well.

Define WS_LOG_DOMAIN. Allows for more targeted showing/hiding of log messages. Use ws_warning() instead of g_warning() to leverage.

Correct spelling of MAX_SSL_VERSION() macro.

Run script through Wireshark's tools/convert-glib-types.py to replace GLib-specific types with C99 standard types (ref Wireshark #19116).

Make every global variable and every function (except the register and handoff functions) static, since none of them are used outside of this plugin.

Remove constant HFIDS and instead define interesting_hfids[] as having an indefinite number of elements. Use the array_length() macro instead for determining the size of the array. It's still calculated at compile time, and removes a potential source of bugs if elements are added to or removed from interesting_hfids[] in future changes.

Add g_strfreev() calls to correspond with each call to g_strsplit(), which specifies in its
documentation that its return value must be freed.

@vlvkobal
Copy link
Member

Hi David, the tests fail. Please look at

tshark: Dissector bug: register_subtree_array: subtree item type (ett_...) not -1 !
This is a development error: Either the subtree item type has already been assigned or was not initialized to -1.

Some minor tweaks to the coding style of the Wireshark dissector to
bring it closer to the standard practices used in the Wireshark project
itself. All changes are compatible with Wireshark 4.6 and should be with
4.4 as well.

Define `WS_LOG_DOMAIN`. Allows for more targeted showing/hiding of
[log messages](https://www.wireshark.org/docs/wsdg_html/#ChSrcLogging).
Use `ws_warning()` instead of `g_warning()` to leverage.

Correct spelling of `MAX_SSL_VERSION()` macro.

Run script through Wireshark's `tools/convert-glib-types.py` to replace
GLib-specific types with C99 standard types (ref Wireshark
[#19116](https://gitlab.com/wireshark/wireshark/-/issues/19116)).

Run script through Wireshark's `tools/convert-proto-init.py` to remove
initialization of static variables, particularly for `hf_*` fields.
Unlike variables within functions, static variables are implicitly
initialized to 0 by the compiler if not otherwise initialized, so this
removes some overhead of static initialization.

Make every global variable and every function (except the register and
handoff functions) static, since none of them are used outside of this
plugin.

Remove constant `HFIDS` and instead define `interesting_hfids[]` as
having an indefinite number of elements. Use the `array_length()` macro
instead for determining the size of the array. It's still calculated at
compile time, and removes a potential source of bugs if elements are
added to or removed from `interesting_hfids[]` in future changes.

Add `g_strfreev()` calls to correspond with each call to `g_strsplit()`,
which specifies in its
[documentation](https://docs.gtk.org/glib/func.strsplit.html) that its
return value must be freed.
@Boolean263
Copy link
Contributor Author

Ah, I see. The tests use Wireshark 4.2.2 which predates the change around initializing static variables. I missed this because I've been doing my in-tree dev work against Wireshark's master branch, which is closest to the Wireshark 4.6.0 release.

That part should be easy enough to revert while keeping the rest of the changes, but I'll test it against 4.2.2 to see if there are any other modernisms that would break that version. (I'm assuming you wouldn't test that version if you were able to migrate to something newer.)

Some minor tweaks to the coding style of the Wireshark dissector to
bring it closer to the standard practices used in the Wireshark project
itself. All changes are compatible with Wireshark 4.6 and should be with
4.4 as well.

Define `WS_LOG_DOMAIN`. Allows for more targeted showing/hiding of
[log messages](https://www.wireshark.org/docs/wsdg_html/#ChSrcLogging).
Use `ws_warning()` instead of `g_warning()` to leverage.

Correct spelling of `MAX_SSL_VERSION()` macro.

Run script through Wireshark's `tools/convert-glib-types.py` to replace
GLib-specific types with C99 standard types (ref Wireshark
[#19116](https://gitlab.com/wireshark/wireshark/-/issues/19116)).

Make every global variable and every function (except the register and
handoff functions) static, since none of them are used outside of this
plugin.

Remove constant `HFIDS` and instead define `interesting_hfids[]` as
having an indefinite number of elements. Use the `array_length()` macro
instead for determining the size of the array. It's still calculated at
compile time, and removes a potential source of bugs if elements are
added to or removed from `interesting_hfids[]` in future changes.

Add `g_strfreev()` calls to correspond with each call to `g_strsplit()`,
which specifies in its
[documentation](https://docs.gtk.org/glib/func.strsplit.html) that its
return value must be freed.
@vlvkobal vlvkobal merged commit dad8618 into FoxIO-LLC:main Nov 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants