Re: strippwd patch against CVS20041108

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

Hello, Leonard!

Please answer to mc-devel gnome org only, my mailbox is near to my disk quota.

> I do have some questions about add_new_entry_cmd() in its current form.
> Comments inlined in C99 style comments.
> 
> static void add_new_entry_cmd (void)
> {
>     char *title, *url, *to_free;
>     int ret;
> 
>     /* Take current directory as default value for input fields */
>     to_free = title = url = strip_password (g_strdup
> (current_panel->cwd), 1);
> 
> // to_free, title and url now all point to the same string, right?

Yes.

> // They are not all duplicates?

Yes if I parse this right. Well, add_new_entry_input() use title and url in two
ways: 1) it use their string values to display
2) it use their addresses to return g_strdup()ed user input

>     ret = add_new_entry_input (_("New hotlist entry"), _("Directory
> label"),
> 			       _("Directory path"), "[Hotlist]", &title, &url);
>     g_free (to_free);
> // So this g_free() frees that string which is now invalid

Right. Let user does not cancel query_dialog: title and url point now to different memory areas allocated in query_dialog()
to_free != url
to_free != title
title != url

>     if (!ret)
> 	return;
>     if (!title || !*title || !url || !*url) {
> 	g_free (title);
> 	g_free (url);
> // But here you still g_free() both of them

To fix memory leaks.

> 	return;
>     }

And return.
 
>     if (ret == B_ENTER || ret == B_APPEND)
> 	add2hotlist (title, url, HL_TYPE_ENTRY, HL_AFTER_CURRENT);
>     else
> 	add2hotlist (title, url, HL_TYPE_ENTRY, HL_BEFORE_CURRENT);
> // And here you pass them as arguments.

Yes, they now point to g_strdup()ped user input.

>     hotlist_state.modified = 1;
> }

-- 
Regards,
Andrew V. Samoilov.