Wireshark-dev: Re: [Wireshark-dev] Small performance improvements to packet-http.c
Kaul,
At the very least, the change of printf("%s", string) -> printf(string)
is harmful. For example, in the following chunk:
@@ -798,7 +782,7 @@
* request or reply.
*/
proto_tree_add_text(http_tree, tvb, offset,
- next_offset - offset, "%s",
+ next_offset - offset,
tvb_format_text(tvb, offset, next_offset - offset));
offset = next_offset;
break;
The string returned from tvb_format_text() may well contain printf-style
format specifiers, like "%s". In that case, proto_tree_add_text() will
attempt to fill them (which is wrong!) and moreover, in so doing, it will
try to dereference a junk on the stack which happened just above the
function arguments. Most probably, this will result in a segmentation
fault.
Best regards,
Alexey Neyman.
On Wednesday 07 November 2007 10:56:12 Kaul wrote:
> Attached please find an updated version of the patch (diff'ed against
> SVN 23387). It removes (as discussed below) the redundant check for
> request/reply and also changes some printf("%s", string) ->
> printf(string).
>
> I'd be happy to get comments on it, especially if it breaks something.
> It would be great if it could make its way to 0.99.7, but I'll
> understand if it'll misses it.
>
> TIA,
> Y.
>
> On Nov 5, 2007 9:15 AM, Kaul <mykaul@xxxxxxxxx> wrote:
> > Somewhat inspired by the performance improvements to tvbuff, I've
> > made some small performance improvements to packet-http.c:
> > 1. In the most common cases 'GET ', 'POST', 'HTTP' - compare them
> > against the 32bit value of those strings, instead of strncmp(). I
> > reckon in most cases it'll be used, and there won't be need for
> > longer comparison paths. 2. Changed the order of the cases in some
> > switch() statements, to make the common cases first.
> > 3. Changed a look up in an array to have the strings length
> > statically stored in the array, instead of dynamically strlen()'ed
> > every time.
> >
> > I'm sure there's more room for improvement (I need to fix the fact it
> > goes over the first line of request/reply needlessly twice, trying to
> > figure out if it's a request or response, again), but that's a start
> > - I wanted to get feedback that I didn't break anything (as I suspect
> > I might have, especially with segmented or fragmented requests).
> >
> > Comments are welcome,
> > Y.
