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

[Hans-Peter D. <hpd...@gm...>]

Hi,

On 16:49 Sat 17 Sep     , Daniel Carl wrote:
> Hi!
> 
> I looked through the code and found places where memory wasn't free()d. I'm a
> C-beginner so maybe some of the free()s aren't needed.

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.

> -    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

> @@ -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.


I ran vimprobable with your patches in valgrind and it seems that most
memory leaks have been plugged. I couldn't test everything, because
webkit's GC and valgrind don't seem to like each other, so the whole
thing crashed as soon as the GC kicked in.

However, I found two other leaks. I will create a patch for them tomorrow.

Regards,
HP