hotlist.c, code beautification

[Roland Illig <roland illig gmx de>]

Hi,

I have rewritten parts of hotlist.c to be easier to understand. (I hope 
it really will.) I think the patched version makes it clear which 
function is responsible for freeing the allocated memory. These are the 
changes I made.

add2hotlist gets "const char *" arguments instead of "char *". It will 
then create local copies with g_strdup() as needed. This way it will not 
be possible after the return of add2hotlist() to alter the strings that 
are saved. You can also be sure that add2hotlist does not free the 
strings you are passing.

In the function add_new_entry_input the "inout" r1 and r2 arguments are 
splitted into "in" arguments and "out" arguments. The "in" arguments are 
not modified by the function and have nothing to do with the "out" 
arguments, so why should they be passed using the same variable?

The main changes are in the function add_new_entry_cmd. All variables 
that probably contain allocated strings are explicitly initialized. This 
is especially important for title and url, as the function 
add_new_entry_input does not give any guarantees about the return value 
in case of an error. Each of these variables will never be assigned more 
than one value. After all the work has been done, they are freed.

Roland

Index: src/hotlist.c
===================================================================
RCS file: /cvsroot/mc/mc/src/hotlist.c,v
retrieving revision 1.75
diff -u -p -u -p -r1.75 hotlist.c
--- src/hotlist.c	10 Nov 2004 11:02:23 -0000	1.75
+++ src/hotlist.c	10 Nov 2004 17:19:59 -0000
@@ -735,7 +735,7 @@ enum {
 };
 
 static struct hotlist *
-add2hotlist (char *label, char *directory, enum HotListType type, int pos)
+add2hotlist (const char *label, const char *directory, enum HotListType type, int pos)
 {
     struct hotlist *new;
     struct hotlist *current = NULL;
@@ -753,8 +753,8 @@ add2hotlist (char *label, char *director
     new = new_hotlist ();
 
     new->type      = type;
-    new->label     = label;
-    new->directory = directory;
+    new->label     = g_strdup (label);
+    new->directory = g_strdup (directory);
     new->up	   = current_group;
 
     if (!current_group->head) { /* first element in group */
@@ -828,7 +828,8 @@ static void add_widgets_i18n(QuickWidget
 
 static int
 add_new_entry_input (const char *header, const char *text1, const char *text2,
-		     const char *help, char **r1, char **r2)
+		     const char *help, const char *r1_default, const char *r2_default,
+                     char **r1, char **r2)
 {
 #define RELATIVE_Y_BUTTONS	4
 #define RELATIVE_Y_LABEL_PTH	3
@@ -879,8 +880,8 @@ add_new_entry_input (const char *header,
     Quick_input.i18n  = 0;
     quick_widgets [6].text = text1;
     quick_widgets [4].text = text2;
-    quick_widgets [5].text = *r1;
-    quick_widgets [3].text = *r2;
+    quick_widgets [5].text = r1_default;
+    quick_widgets [3].text = r2_default;
 
     for (i = 0; i < 7; i++)
 	quick_widgets [i].y_divisions = lines1+lines2+7;
@@ -904,23 +905,18 @@ add_new_entry_input (const char *header,
 
 static void add_new_entry_cmd (void)
 {
-    char *title, *url, *to_free;
+    char *cwd = NULL, *title = NULL, *url = NULL;
     int ret;
 
     /* Take current directory as default value for input fields */
-    to_free = title = url = strip_password (g_strdup (current_panel->cwd), 1);
+    cwd = strip_password (g_strdup (current_panel->cwd), 1);
 
     ret = add_new_entry_input (_("New hotlist entry"), _("Directory label"),
-			       _("Directory path"), "[Hotlist]", &title, &url);
-    g_free (to_free);
+                               _("Directory path"), "[Hotlist]",
+                               cwd, cwd, &title, &url);
 
-    if (!ret)
-	return;
-    if (!title || !*title || !url || !*url) {
-	g_free (title);
-	g_free (url);
-	return;
-    }
+    if (!ret || !title || !*title || !url || !*url)
+        goto cleanup;
 
     if (ret == B_ENTER || ret == B_APPEND)
 	add2hotlist (title, url, HL_TYPE_ENTRY, HL_AFTER_CURRENT);
@@ -928,6 +924,11 @@ static void add_new_entry_cmd (void)
 	add2hotlist (title, url, HL_TYPE_ENTRY, HL_BEFORE_CURRENT);
 
     hotlist_state.modified = 1;
+
+cleanup:
+    g_free (cwd);
+    g_free (title);
+    g_free (url);
 }
 
 static int