Wireshark · Wireshark-dev: Re: [Wireshark-dev] Why isn't this a bug (2)?

[Jaap Keuter <jaap.keuter@xxxxxxxxx>]

On 07/10/2011 08:02 PM, Chris Maynard wrote:
> Jaap Keuter<jaap.keuter@...>  writes:
>
> > static gboolean
> > check_msrp_header(tvbuff_t *tvb)
> > {
> > ...
> >       linelen = tvb_find_line_end(tvb, 0, -1,&next_offset, FALSE);
> >       /* Find the first SP */
> >       space_offset = tvb_find_guint8(tvb, 0, -1, ' ');
> >
> > ...
> > }
> >
> > Why find the line length first, then to start searching to the end of the TVB?
> > A comment suggests that tvb_get_ptr() was used previously, so this might be a
> > botched rework of that code?
>
> Currently neither linelen nor next_offset are used in that function after
> assignment, so there's clearly something amiss here.  To me, it looks like
> "linelen = tvb_find_line_end(tvb, 0, -1,&next_offset, FALSE);" is just
> superfluous, unneeded code that was probably due to "copy-and-paste" or rework
> by the original author, as the code has been that way since day 1.  See
> http://anonsvn.wireshark.org/viewvc/trunk/epan/dissectors/packet-msrp.c?revision=14491&view=markup

Yes, this could be coding away from tvb_get_ptr() (which is a Good Thing(sm)). 
Still, looking at the record format, it's not bad to use the record length when 
checking the correctness, so I think changing it to this will do:

       space_offset = tvb_find_guint8(tvb, 0, linelen, ' ');
...
       space_offset = tvb_find_guint8(tvb, token_2_start, linelen, ' ');

Anders?

Thanks,
Jaap