Wireshark mailing list archives
Re: Why isn't this a bug (2)?
From: Jaap Keuter <jaap.keuter () xs4all nl>
Date: Sun, 10 Jul 2011 23:20:13 +0200
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 ___________________________________________________________________________ Sent via: Wireshark-dev mailing list <wireshark-dev () wireshark org> Archives: http://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev mailto:wireshark-dev-request () wireshark org?subject=unsubscribe
Current thread:
- Why isn't this a bug (2)? Jaap Keuter (Jul 08)
- Re: Why isn't this a bug (2)? Chris Maynard (Jul 10)
- Re: Why isn't this a bug (2)? Jaap Keuter (Jul 10)
- Re: Why isn't this a bug (2)? Anders Broman (Jul 10)
- Re: Why isn't this a bug (2)? Jaap Keuter (Jul 10)
- Re: Why isn't this a bug (2)? Chris Maynard (Jul 10)