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@wireshark.org> Archives: https://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe
___________________________________________________________________________ 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-requ...@wireshark.org?subject=unsubscribe
