Wireshark-dev: Re: [Wireshark-dev] a bug or a feature
From: Michael Mann <mmann78@xxxxxxxxxxxx>
Date: Thu, 4 Nov 2021 23:17:23 +0000 (UTC)
One of the other main reasons is that some users switch between different versions of Wireshark and if a preference has been "removed" (not registered as obsolete) the preferences file will be modified to have that preference removed, which would result in it being restored to a "default" value when switching to a different Wireshark version. The "obsolete" tag just leaves the preference alone and allows users to freely switch between Wireshark versions.
-----Original Message-----
From: João Valverde <j@xxxxxx>
To: wireshark-dev@xxxxxxxxxxxxx
Sent: Thu, Nov 4, 2021 7:46 am
Subject: Re: [Wireshark-dev] a bug or a feature
On 03/11/21 10:50, Zoran Bošnjak wrote:
> 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.
I assume the intention is to keep the obsolete call indefinitely. It's
not intended to be re-introduced with a different behavior either. I
think you raise some valid points but why is the option "keep the call"
wrong? A more wordy name might be "no_longer_actively_used" instead of
deprecated. Does that fix whatever is wrong?
> 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
> ___________________________________________________________________________
> Sent via: Wireshark-dev mailing list <wireshark-dev@xxxxxxxxxxxxx>
> Archives: https://www.wireshark.org/lists/wireshark-dev
> Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
> mailto:wireshark-dev-request@xxxxxxxxxxxxx?subject=unsubscribe
___________________________________________________________________________
Sent via: Wireshark-dev mailing list <wireshark-dev@xxxxxxxxxxxxx>
Archives: https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
mailto:wireshark-dev-request@xxxxxxxxxxxxx?subject=unsubscribe
From: João Valverde <j@xxxxxx>
To: wireshark-dev@xxxxxxxxxxxxx
Sent: Thu, Nov 4, 2021 7:46 am
Subject: Re: [Wireshark-dev] a bug or a feature
On 03/11/21 10:50, Zoran Bošnjak wrote:
> 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.
I assume the intention is to keep the obsolete call indefinitely. It's
not intended to be re-introduced with a different behavior either. I
think you raise some valid points but why is the option "keep the call"
wrong? A more wordy name might be "no_longer_actively_used" instead of
deprecated. Does that fix whatever is wrong?
> 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
> ___________________________________________________________________________
> Sent via: Wireshark-dev mailing list <wireshark-dev@xxxxxxxxxxxxx>
> Archives: https://www.wireshark.org/lists/wireshark-dev
> Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
> mailto:wireshark-dev-request@xxxxxxxxxxxxx?subject=unsubscribe
___________________________________________________________________________
Sent via: Wireshark-dev mailing list <wireshark-dev@xxxxxxxxxxxxx>
Archives: https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
mailto:wireshark-dev-request@xxxxxxxxxxxxx?subject=unsubscribe
- References:
- [Wireshark-dev] a bug or a feature
- From: Zoran Bošnjak
- Re: [Wireshark-dev] a bug or a feature
- From: João Valverde
- [Wireshark-dev] a bug or a feature
- Prev by Date: Re: [Wireshark-dev] a bug or a feature
- Next by Date: [Wireshark-dev] Lint for Qt Slots/Signals
- Previous by thread: Re: [Wireshark-dev] a bug or a feature
- Next by thread: [Wireshark-dev] Lint for Qt Slots/Signals
- Index(es):