There are 2 other files that I believe also have tvb_get_string()-related
memory leaks, but they are somewhat confusing to me, and so I would appreciate
it if someone could take a look at them.
1) packet-ldap.c:dissect_ldap_Mechanism(): The comment starting at line 1160
states the following:
/*
* If the mechanism in this request is an empty string (which is
* returned as a null pointer), use the saved mechanism instead.
* Otherwise, if the saved mechanism is an empty string (null),
* save this mechanism.
*/
But that doesn't look like what's going on. Instead, it looks like if the
mechanism is NULL then it is assigned to the saved mechanism ... which is never
used, so what's the point? And if it's NOT NULL then the saved mechanism is
freed and assigned to the new mechanism regardless of whether the saved
mechanism was NULL or not. Since the code doesn't match the comment, I'm
confused. And I'm also not entirely sure if there is a possible
tvb_get_string() memory leak here or not.
2) packet-wsp.c: At line 1281, there is a nice NOTE warning about ensuring to
call g_free() since tvb_get_stringz() returns g_malloc()ed memory. There is
actually only 1 instance of tvb_get_stringz(); however, it is called from the
get_text_string() macro. get_text_string(), in turn, is called by a bunch of
other macros, such as get_token_text(), get_extension_media(),
get_text_value(), get_quoted_string(), get_uri_value() and get_version_value().
Because all of those therefore end up calling tvb_get_stringz(), I believe we
need to see some g_free()'s after all of them.
This doesn't look like the case, but before I went to try to fix them, I
noticed this confusing code/comment at line 2243:
get_token_text(val_str, tvb, off, len, ok); \
/* As we're using val_str, it is automatically g_free()d */ \
There is no g_free() following that code, so I think there is a leak here and
that the comment can't be true ... can it? Am I missing something here? I
stopped looking for leaks at this point for this dissector until I can clear up
this confusion.
Thanks.
- Chris
> -Original Message-
> From: wireshark-commits-boun...@wireshark.org [mailto:wireshark-
> commits-boun...@wireshark.org] On Behalf Of cmayn...@wireshark.org
> Sent: Wednesday, December 28, 2011 11:37 AM
> To: wireshark-comm...@wireshark.org
> Subject: [Wireshark-commits] rev 40312: /trunk/
> /trunk/epan/dissectors/: packet-bt-dht.c packet-gopher.c packet-
> gsm_ipa.c packet-meta.c packet-mux27010.c packet-nfs.c packet-rdp.c
> packet-sametime.c packet-ua.c packet-xtp.c ...
>
> http://anonsvn.wireshark.org/viewvc/viewvc.cgi?view=rev&revision=40312
>
> User: cmaynard
> Date: 2011/12/28 08:36 AM
>
> Log:
> Fix memory leaks involving tvb_get_string[z]().
>
> Directory: /trunk/epan/dissectors/
> ChangesPath Action
> +5 -5 packet-bt-dht.c Modified
> +1 -0 packet-gopher.c Modified
> +1 -1 packet-gsm_ipa.c Modified
> +3 -0 packet-meta.cModified
> +7 -5 packet-mux27010.cModified
> +3 -0 packet-nfs.c Modified
> +6 -6 packet-rdp.c Modified
> +1 -1 packet-sametime.cModified
> +4 -4 packet-ua.c Modified
> +1 -0 packet-xtp.c Modified
>
>
> (1 file not shown)
> ___
>
> Sent via:Wireshark-commits mailing list comm...@wireshark.org>
> Archives:http://www.wireshark.org/lists/wireshark-commits
> Unsubscribe: https://wireshark.org/mailman/options/wireshark-commits
> mailto:wireshark-commits-
> requ...@wireshark.org?subject=unsubscribe
--
CONFIDENTIALITY NOTICE: The information contained in this email message is
intended only for use of the intended recipient. If the reader of this message
is not the intended recipient, you are hereby notified that any dissemination,
distribution or copying of this communication is strictly prohibited. If you
have received this communication in error, please immediately delete it from
your system and notify the sender by replying to this email. Thank you.
___
Sent via:Wireshark-dev mailing list
Archives:http://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe
