Wireshark-dev: [Wireshark-dev] a bug or a feature

Date Prev · Date Next · Thread Prev · Thread Next
From: Zoran Bošnjak <zoran.bosnjak@xxxxxx>
Date: Wed, 3 Nov 2021 10:50:04 +0000 (UTC)
Hello wireshark developers,
I would appreciate some clarification about "making preferences obsolete".

As this problem reports (which is also in accordance with README.dissector)
https://gitlab.com/wireshark/wireshark/-/issues/17697
... is to make each removed preference explicitely obsolete with a call to
prefs_register_obsolete_preference.

By calling this function, the misconfiguration warning is suppressed.

The reported issue is easy to fix and also easy to modify as suggested by reviewer, but I have some general questions about the preferences. Also, I am not sure if this fix is really wanted, but rather a register_obsolete needs a review.

1. The original warning looks like a correct behaviour. It's informative and it correctly indicates a misconfiguration to the user.
Why would one want to suppress the warning?

2. If each such warning is to be suppressed, then why logging it in the first place?

3. The mechanism is fragile. It only handles the change once. Once the preference is made obsolete, the call to 
prefs_register_obsolete_preference remains in the code forever. This raises the question:
How do you re-introduce the same preference back, in particular:
Do you remove the register_obsolete call from the code or keep it in the code?
If it's "remove the call": then it was unnecessary in the first place, because the call is exactly
the negation of a preference being currently present (which we already known from the rest of the code)
If it's "to keep the call": then it's wrong, because the preference is present and the call says
it's obsolete.

4. A patch to the reported problem was attempted
https://gitlab.com/zoranbosnjak/wireshark/-/commit/3e6fe4e0c3eca2b21151176c988ea36adac10b40
In this case, there is a closed set (256) of possible preferences, so that all not-supported categories can be made obsolete in advance, which effectively suppresses the warnings, regardless of the supported category set.
A reviewer suggested a "lean" approach which suffers the remove/add problem and it requires hardcoding a category list.
If the reported issue really is a problem, why "lean" approach? I can not forsee any noticable burden for the preference engine. It's like trying to optimize for assembler, rather than source C code (the dissector C code is generated in this case and is therefore at the lower layer which does not need such optimization, at the cost of the real source code being suboptimal and in this case even (potentially) wrong). Please clarify the reasoning behind this.

regards,
Zoran