Wireshark-dev: Re: [Wireshark-dev] Should we check value_strings for NULL termination while reg
From: "ronnie sahlberg" <ronniesahlberg@xxxxxxxxx>
Date: Fri, 22 Sep 2006 22:21:54 +0000
That would be an awful lot of work since we would need to modify (add
registration of) all value_strings.

Barring we could convince the coverity guys to add a check "make sure
all arrays of the type value_string are terminated with {0,NULL}"
but that wouldnt help out of tree dissectors   nor can we rely on
coverity being available to us forever.


If we are making these changes  we might also at the same time try to
address the "linear search" property for the existing struct.
While most value_strings are small  there are some very very large
ones that are used very frequently such as the SMB NT status code
array.

Binary trees have both good and bad properties. They would be good for
managing these few very large value_strings efficiantly  but since
most of the value_strings are very small there would be a massive
memory overhead for these small value_strings where a linear search
would be quite sufficient.

(we could change the accessor/search function for value strings to
still keep it as a linear search array   but everytime something has
been looked up,  we swap some pointers and let the "found" entry
bubble upwards in the array to get it to automatically make sure at
runtime that the frequently looked up entries are always near the
start of the array and thus quick to find with a linear search)



Well   an easy way to make sure all of them are properly terminated
with a {0,NULL} is by using variadic macros.
Unfortunately not standard until c99   but been part of GCC forever.

#define VALUE_STRING(name, ...)		\
value_string_t name[] = {		\
	__VA_ARGS__			\
	{0, NULL}			\
};

VALUE_STRING(vs,
 {1, "foo"},
 {2, "bar"},
)


Does VC6 support variadic macros?




On 9/20/06, Ulf Lamping <ulf.lamping@xxxxxx> wrote:
Hi List!

It seems to be a common mistake to forget the terminating zero entry in a
value_string, I've done this myself before and it's hard to track it down if
you don't have a clue what's going wrong.

Even worse, this mistake might not make any problems for a long time as
usually the values rushing in will be in the value_string (I've found my
mistake by fuzz-testing).

We could automatically check the termination of the value_string (at least
for the registered values) while registering the hf_ values. This will
indicate the problem a lot earlier and at the place where it's caused (and
not much, much later).

Of cause this has a drawback: Iterating trough all value_strings at
registration will take a little bit of time at startup (didn't tried so
don't know how long this will take).

What do others think: Is the added time overhead worth adding some extra
robustness or are there other mechanisms possible to detect these kind of
errors (e.g. static code analysis)?

Regards, ULFL
_____________________________________________________________________
Der WEB.DE SmartSurfer hilft bis zu 70% Ihrer Onlinekosten zu sparen!
http://smartsurfer.web.de/?mc=100071&distributionid=000000000066

_______________________________________________
Wireshark-dev mailing list
Wireshark-dev@xxxxxxxxxxxxx
http://www.wireshark.org/mailman/listinfo/wireshark-dev