/hg/icedtea-web: Fixed two memory leaks in C++ side of plugin

[Adam Domurad]

Hey Pavel. As I mentioned in my RFC post, I stumbled upon one of the
memory leaks, and then did a grep of (alloc\(|free\() which highlighted
malloc/free pairs, or lack-thereof. This wasn't anything extensive and
by no means exhaustive.

The copy of the original string was not necessary. The string is
immutable in this context, and immediately copied by the consumer
anyway.

On Mon, 2012-06-18 at 11:32 +0200, Pavel Tisnovsky wrote:
> Hi Adam,
> 
> your fixes looks ok for me, thank you.
> 
> I just have two questions:
> 
> 1) did you use valgrind or similar tool to found (possibly all) memory leaks? Or did you simply look at the code?
> 
> 2) the second fix (msg/message): was there any reason to use a copy of original message string (more thread modifying
> the message str. or something similar)?
> 
> Cheers,
> Pavel
> 
> adomurad at icedtea.classpath.org wrote:
> > changeset 24722d62f53c in /hg/icedtea-web
> > details: http://icedtea.classpath.org/hg/icedtea-web?cmd=changeset;node=24722d62f53c
> > author: Adam Domurad <adomurad at redhat.com>
> > date: Fri Jun 15 15:27:12 2012 -0400
> > 
> > 	Fixed two memory leaks in C++ side of plugin
> > 
> > 
> > diffstat:
> > 
> >  ChangeLog                              |   9 +++++++++
> >  plugin/icedteanp/IcedTeaNPPlugin.cc    |   3 +++
> >  plugin/icedteanp/IcedTeaPluginUtils.cc |  12 ++++--------
> >  3 files changed, 16 insertions(+), 8 deletions(-)
> > 
> > diffs (69 lines):
> > 
> > diff -r 1819a9bbd922 -r 24722d62f53c ChangeLog
> > --- a/ChangeLog	Thu Jun 14 11:11:01 2012 -0400
> > +++ b/ChangeLog	Fri Jun 15 15:27:12 2012 -0400
> > @@ -1,3 +1,12 @@
> > +2012-06-15  Adam Domurad  <adomurad at redhat.com>
> > +
> > +	Fixed two memory leaks
> > +	* plugin/icedteanp/IcedTeaNPPlugin.cc
> > +	(consume_message): Call to g_strsplit matched with call to g_strfreev.
> > +	* plugin/icedteanp/IcedTeaPluginUtils.cc
> > +	(post): Removed copy of string, which assumed consumer freed string
> > +	(which was not true and not always possible)
> > +
> >  2012-06-11  Danesh Dadachanji  <ddadacha at redhat.com>
> >  
> >  	PR855: AppletStub getDocumentBase() doesn't return full URL
> > diff -r 1819a9bbd922 -r 24722d62f53c plugin/icedteanp/IcedTeaNPPlugin.cc
> > --- a/plugin/icedteanp/IcedTeaNPPlugin.cc	Thu Jun 14 11:11:01 2012 -0400
> > +++ b/plugin/icedteanp/IcedTeaNPPlugin.cc	Fri Jun 15 15:27:12 2012 -0400
> > @@ -1303,11 +1303,14 @@
> >          g_free(cookie_info);
> >          cookie_info = NULL;
> >        }
> > +      g_strfreev (parts);
> > +      parts = NULL;
> >      }
> >    else
> >      {
> >          g_print ("  Unable to handle message: %s\n", message);
> >      }
> > +
> >  }
> >  
> >  void get_instance_from_id(int id, NPP& instance)
> > diff -r 1819a9bbd922 -r 24722d62f53c plugin/icedteanp/IcedTeaPluginUtils.cc
> > --- a/plugin/icedteanp/IcedTeaPluginUtils.cc	Thu Jun 14 11:11:01 2012 -0400
> > +++ b/plugin/icedteanp/IcedTeaPluginUtils.cc	Fri Jun 15 15:27:12 2012 -0400
> > @@ -1124,27 +1124,23 @@
> >  void
> >  MessageBus::post(const char* message)
> >  {
> > -	char* msg = (char*) malloc(sizeof(char)*strlen(message) + 1);
> >  	bool message_consumed = false;
> >  
> > -	// consumer frees this memory
> > -	strcpy(msg, message);
> > -
> >  	PLUGIN_DEBUG("Trying to lock %p...\n", &msg_queue_mutex);
> >  	pthread_mutex_lock(&subscriber_mutex);
> >  
> > -    PLUGIN_DEBUG("Message %s received on bus. Notifying subscribers.\n", msg);
> > +    PLUGIN_DEBUG("Message %s received on bus. Notifying subscribers.\n", message);
> >  
> >      std::list<BusSubscriber*>::const_iterator i;
> >      for( i = subscribers.begin(); i != subscribers.end() && !message_consumed; ++i ) {
> > -    	PLUGIN_DEBUG("Notifying subscriber %p of %s\n", *i, msg);
> > -    	message_consumed = ((BusSubscriber*) *i)->newMessageOnBus(msg);
> > +    	PLUGIN_DEBUG("Notifying subscriber %p of %s\n", *i, message);
> > +    	message_consumed = ((BusSubscriber*) *i)->newMessageOnBus(message);
> >      }
> >  
> >      pthread_mutex_unlock(&subscriber_mutex);
> >  
> >      if (!message_consumed)
> > -    	PLUGIN_DEBUG("Warning: No consumer found for message %s\n", msg);
> > +    	PLUGIN_DEBUG("Warning: No consumer found for message %s\n", message);
> >  
> >      PLUGIN_DEBUG("%p unlocked...\n", &msg_queue_mutex);
> >  }
>