Wireshark mailing list archives
const'ness of value_string_ext
From: Kevin Cox <kevincox () kevincox ca>
Date: Thu, 24 Jul 2014 16:35:07 -0400
Hello All, While working on the new Ceph dissector I made a mistake using value_string_ext (herein evs) where I declared them 'const' which was causing an error when they were put in a read-only segment of the executable. Thankfully Bill Meier was able to figure out why it was crashing on his system but it made me curious why this error was possible without compiler errors or warnings. After looking into epan/value_string.{h,c} I noticed that all of the evs methods took 'const value_string_ext*' even though many of them may end up writing to the evs. This all boils down to the "virtual" method '_vs_match2' which may contain '_try_val_to_str_ext_init' which casts away the const'ness and writes to the evs. There is a comment inside this function explaining why the const is casted away however I believe that this isn't the proper solution. The comment discusses casting inside the function versus casting the function pointer when assigning it to '_vs_match2'. I would argue that the correct solution is actually to change the prototype of '_vs_match2' from: const value_string *(*)(const guint32, const value_string_ext*) to const value_string *(*)(const guint32, value_string_ext*) then change all of the evs functions to take a non-const evs. This would convey the actual signature of the functions and prevent this type of error in the future. However, the problem with this approach is that when declaring hf's and probably other places around the source 'const void*'s are used to handle evs. So changing these functions would cause large changes to the code. There a a number of solutions of varying completeness. 0: Leave as is. Pros: - Easy. Cons: - The API lies and can invoke undefined behaviour unless you know. - No compiler checking. 1: Change all of the evs functions and add the const-cast further up the stack. Pros: - The evs functions don't lie and people will get errors if they use an evs directly. - The evs API is now correct. - Can slowly start to move the rest of the code to be const-correct. Cons: - There is still an issue in a different part of the code. - Not full type safety. 2: Change everything. Pros: - Full compiler error checking and type safety. Cons: - Hard - May change dissector API. I was wondering what everyone else thought and what should be done to improve the safety of this code. Cheers, Kevin
Attachment:
signature.asc
Description: OpenPGP digital signature
___________________________________________________________________________ Sent via: Wireshark-dev mailing list <wireshark-dev () wireshark org> Archives: http://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev mailto:wireshark-dev-request () wireshark org?subject=unsubscribe
Current thread:
- const'ness of value_string_ext Kevin Cox (Jul 24)
- Re: const'ness of value_string_ext Evan Huus (Jul 24)
- Re: const'ness of value_string_ext Kevin Cox (Jul 24)
- Re: const'ness of value_string_ext darkjames-ws (Jul 25)
- Re: const'ness of value_string_ext Kevin Cox (Jul 25)
- Re: const'ness of value_string_ext Evan Huus (Jul 24)