Re: [Vimprobable-users] [PATCH] Possible fix for crash on :echo

[Daniel C. <dan...@gm...>]

On Wed, Oct 05, 2011 at 01:31:42AM +0200, Hans-Peter Deifel wrote:
> Hi,
> Unless I've overseen something, your patches are good. Only a few
> comments:
> 
> >  setup_settings() {
> >      WebKitWebSettings *settings = (WebKitWebSettings*)webkit_web_settings_new();
> >      SoupURI *proxy_uri;
> > -    char *filename, *new;
> > +    char *filename, *file_url, *new;
> >      int  len;
> >  
> >      session = webkit_get_default_session();
> > @@ -2140,8 +2151,9 @@ setup_settings() {
> >      g_object_set(G_OBJECT(settings), "enable-java-applet", enableJava, NULL);
> >      g_object_set(G_OBJECT(settings), "enable-page-cache", enablePagecache, NULL);
> >      filename = g_strdup_printf(USER_STYLESHEET);
> 
> filename should be freed.

Yes it should, but if I free it vimprobable crashes and I have no idea why.
Does it work for you if you free filename?

> > -    filename = g_strdup_printf("file://%s", filename);
> > -    g_object_set(G_OBJECT(settings), "user-stylesheet-uri", filename, NULL);
> > +    file_url = g_strdup_printf("file://%s", filename);
> > +    g_object_set(G_OBJECT(settings), "user-stylesheet-uri", file_url, NULL);
> > +    g_free(file_url);
> >      g_object_set(G_OBJECT(settings), "user-agent", useragent, NULL);
> >      g_object_get(G_OBJECT(settings), "zoom-step", &zoomstep, NULL);
> >      webkit_web_view_set_settings(webview, settings);
> 
> A few lines from here, 'new' is allocated and not freed

This is right, can you do that with your patch for the other two leaks?

> > @@ -2397,6 +2409,7 @@ main(int argc, char *argv[]) {
> >  
> >  	    feedback_str = g_strdup_printf("Config file '%s' doesn't exist", cfile);
> >  	    give_feedback(feedback_str);
> > +        g_free(feedback_str);
> 
> Indentation is somehow broken here. Not your fault, but still strange.

We should fix code indentation with an extra patch, by the way the whitespace
errors could be fixed (trailing whitespace).

Daniel