Re: [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 ...

["Maynard, Chris" <Christopher.Maynard () GTECH COM>]

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-bounces () wireshark org [mailto:wireshark-
> commits-bounces () wireshark org] On Behalf Of cmaynard () wireshark org
> Sent: Wednesday, December 28, 2011 11:37 AM
> To: wireshark-commits () 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/
>   Changes    Path                 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.c        Modified
>   +7 -5      packet-mux27010.c    Modified
>   +3 -0      packet-nfs.c         Modified
>   +6 -6      packet-rdp.c         Modified
>   +1 -1      packet-sametime.c    Modified
>   +4 -4      packet-ua.c          Modified
>   +1 -0      packet-xtp.c         Modified
>
>
> (1 file not shown)
> _______________________________________________________________________
> ____
> Sent via:    Wireshark-commits mailing list <wireshark-
> commits () wireshark org>
> Archives:    http://www.wireshark.org/lists/wireshark-commits
> Unsubscribe: https://wireshark.org/mailman/options/wireshark-commits
>              mailto:wireshark-commits-
> request () 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 <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