Re: strippwd patch against CVS20041108

["Andrew V. Samoilov" <andrew email zp ua>]

> Hello Andrew,
> 
> On Wed, 2004-11-10 at 11:51, Andrew V. Samoilov wrote:
> > Patch below fixes double free, memory leaks and missed {}.
> 
> Double frees? Please explain.
. . .
    if (ret == B_ENTER || ret == B_APPEND)
        add2hotlist (title, url, HL_TYPE_ENTRY, 2);
    else
        add2hotlist (title, url, HL_TYPE_ENTRY, 1);

    hotlist_state.modified = 1;

    g_free (title);
    g_free (url);
. . .
title and url are g_free()d here and in remove_group()|remove_from_hotlist() after.
 
> And what about
> 
> -    if (!ret || !title || !*title || !url || !*url) {
> +    if (!ret)
> +       return;
> +    if (!title || !*title || !url || !*url) {
> 
> ? title and url should always be g_free()d, because either of them can
> be non NULL even if ret is NULL.

Well, add_new_entry_input() returns 0 if query_dialog() is canceled.
query_dialog() does not strdup() str_result's, so title and url are unchanged
this way.

> If I am not mistaken this should be dropped from the patch. I also don't
> see the use of to_free. What does it actually do?

to_free = strip_password (g_strdup (current_panel->cwd), 1),
so this memory area needs its own g_free().
url and title will be newly allocated strings if user does not cancel
query_dialog().

> enum is nice as it clears up the code and makes the comments in the code
> redundant. Not sure if we should introduce it before 4.6.1 though.

This part is trivial.
 
> > Second part is optimization.
> 
> Even more patches to test in the time of which new patches will have
> emerged. Can we concentrate on fixing the last items and have a release
> please?

They are trivial too, these hunks were produced when I discovered mc crash on
Pavel's report.

There are some dozens of bugs to fix, and some of then are not in TODO
after they were intoduced on mc-devel gnome org 

-- 
Regards,
Andrew V. Samoilov.