Wireshark-dev: Re: [Wireshark-dev] check_col() depcreated?
From: didier <dgautheron@xxxxxxxx>
Date: Thu, 14 May 2009 07:35:53 +0200
Hi,
Le mercredi 13 mai 2009 à 18:36 +0200, Joerg Mayer a écrit :
> On Wed, May 13, 2009 at 04:44:26PM +0200, Jaap Keuter wrote:
> > Indeed this was done to find/eliminate some tricky crashes of  
> > Wireshark. Like with proto_add_*(), which has the requirement for tree! 
> > =NULL removed, the col_*() functions are now hardened against ! 
> > check_col().
> > Still it's not wrong to test for it, and most, if not all, still does.  
> > With the branch of 1.2 on the horizon I would suggest holding off this  
> > 'housekeeping'  issue until we're working on 1.3.
> 
> Hmm, how about removing that function and adding an empty #define for
> it in the .h file and #undefine it in the .c file after including
> the .h-file. If no new problems come up, people may safely remove that stuff
> any time they want and the initial patch isn't big.
check_col() shouldn't be used anymore if col_xxx is called with a
constant ie:

  if (check_col(pinfo->cinfo, COL_PROTOCOL))
    col_set_str(pinfo->cinfo, COL_PROTOCOL, "IP");

But it still useful if there's very slow functions like snprintf,
val_to_str, format_text and so on ie:

  if (check_col(pinfo->cinfo, COL_INFO)) {
    bufpos=0;
    bufpos+=MIN(MAX_BUF_SIZE-bufpos,
                g_snprintf(buf+bufpos, MAX_BUF_SIZE-bufpos, "%s%s",
                val_to_str(opcode, opcode_vals, "Unknown (%u)"),
                (flags&F_RESPONSE)?" response":""));

    ...
    col_add_str(pinfo->cinfo, COL_INFO, buf);
    cinfo = pinfo->cinfo;
  } else {
    cinfo = NULL;
  }


Didier