Jaap Keuter skrev 2011-07-10 23:20:
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?
Looks good, I have checked in that code.
Regards
Anders
Thanks,
Jaap
___________________________________________________________________________
Sent via: Wireshark-dev mailing list <wireshark-dev@xxxxxxxxxxxxx>
Archives: http://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
mailto:wireshark-dev-request@xxxxxxxxxxxxx?subject=unsubscribe
