Wireshark-dev: [Wireshark-dev] Changing the column setting API

From: Jaap Keuter <jaap.keuter@xxxxxxxxx>
Date: Wed, 21 Jan 2009 06:51:26 +0100
Hi,

In the course of fixing bug 2902 (https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=2902) a proposal was made to change the API of the column setting functions.

Currently the user (dissector code) has to check the column before accessing it. So a lot of code looks like this:

  if (check_col(pinfo->cinfo, COL_PROTOCOL))
    col_set_str(pinfo->cinfo, COL_PROTOCOL, <protocol name>);

If the check is not done the code works just as well, but can also cause a crash of Wireshark (hence bug 2902).

This used to be the way proto_tree_add_xxx(tree, ...) had to be handled. First a check for tree!=NULL, then proto_tree_add_xxx() was done. Eventually the check for tree!=NULL was handled by the API, hence no longer needed in the dissector code.

The proposal is to have the check for column status done in the API, i.s.o. making the user responsible for it, like (s)he is now. This should eliminate the problems causing bug 2902 and makes the dissector code looks cleaner.

Does anyone foresee problems with this change?

Thanx,
Jaap