[patch edit/editcmd.c] Code cleanup

[Roland Illig <roland illig gmx de>]

Hi,

I patched the edit/replace dialog to be able to manage the GCC warnings. 
I applied these coding styles:

  * one variable for exactly one job
  * cleanup phase
  * avoid the ?: operator
  * separate input parameters from output parameters
  * add a typedef for commonly used types, especially functions

I also added some comments for better understanding.

As the patch touches important code, I would like it to be reviewed by 
at least one of you. I hope my code is easy to read -- at least this is 
one of my primary goals.

The calls to memmove in line 1822 aren't needed, as atoi ignores leading 
spaces while scanning.

Roland

Index: editcmd.c
===================================================================
RCS file: /cvsroot/mc/mc/edit/editcmd.c,v
retrieving revision 1.115
diff -u -r1.115 editcmd.c
--- editcmd.c	24 Sep 2004 14:57:57 -0000	1.115
+++ editcmd.c	24 Sep 2004 23:32:35 -0000
@@ -1213,7 +1213,10 @@
 }
 
 static void
-edit_replace_dialog (WEdit * edit, char **search_text, char **replace_text, char **arg_order)
+edit_replace_dialog (WEdit * edit, const char *search_default,
+                     const char *replace_default, const char *argorder_default,
+                     /*out*/ char **search_text, /*out*/ char **replace_text,
+                     /*out*/ char **arg_order)
 {
     int treplace_scanf = replace_scanf;
     int treplace_regexp = replace_regexp;
@@ -1265,11 +1268,11 @@
     quick_widgets[7].result = &treplace_whole;
     quick_widgets[8].result = &treplace_case;
     quick_widgets[9].str_result = arg_order;
-    quick_widgets[9].text = *arg_order;
+    quick_widgets[9].text = argorder_default;
     quick_widgets[11].str_result = replace_text;
-    quick_widgets[11].text = *replace_text;
+    quick_widgets[11].text = replace_default;
     quick_widgets[13].str_result = search_text;
-    quick_widgets[13].text = *search_text;
+    quick_widgets[13].text = search_default;
     {
 	QuickDialog Quick_input =
 	{REPLACE_DLG_WIDTH, REPLACE_DLG_HEIGHT, -1, 0, N_(" Replace "),
@@ -1413,8 +1416,10 @@
 /* thanks to  Liviu Daia <daia stoilow imar ro>  for getting this
    (and the above) routines to work properly - paul */
 
+typedef int (*edit_getbyte_fn) (WEdit *, long);
+
 static long
-edit_find_string (long start, unsigned char *exp, int *len, long last_byte, int (*get_byte) (void *, long), void *data, int once_only, void *d)
+edit_find_string (long start, unsigned char *exp, int *len, long last_byte, edit_getbyte_fn get_byte, void *data, int once_only, void *d)
 {
     long p, q = 0;
     long l = strlen ((char *) exp), f = 0;
@@ -1572,7 +1577,7 @@
 
 
 static long
-edit_find_forwards (long search_start, unsigned char *exp, int *len, long last_byte, int (*get_byte) (void *, long), void *data, int once_only, void *d)
+edit_find_forwards (long search_start, unsigned char *exp, int *len, long last_byte, edit_getbyte_fn get_byte, void *data, int once_only, void *d)
 {				/*front end to find_string to check for
 				   whole words */
     long p;
@@ -1596,7 +1601,7 @@
 }
 
 static long
-edit_find (long search_start, unsigned char *exp, int *len, long last_byte, int (*get_byte) (void *, long), void *data, void *d)
+edit_find (long search_start, unsigned char *exp, int *len, long last_byte, edit_getbyte_fn get_byte, void *data, void *d)
 {
     long p;
     if (replace_backwards) {
@@ -1634,13 +1639,14 @@
 {
     va_list ap;
     size_t n;
-    char *q, *p, *s = str, *e = str + size;
+    const char *q, *p;
+    char *s = str, *e = str + size;
     char q1[40];
     char *p1;
     int nargs = 0;
 
     va_start (ap, fmt);
-    p = q = (char *) fmt;
+    p = q = fmt;
 
     while ((p = strchr (p, '%'))) {
 	n = p - q;
@@ -1745,90 +1751,90 @@
 edit_replace_cmd (WEdit *edit, int again)
 {
     static regmatch_t pmatch[NUM_REPL_ARGS];
-    static char *old1 = NULL;
-    static char *old2 = NULL;
-    static char *old3 = NULL;
-    char *exp1 = "";
-    char *exp2 = "";
-    char *exp3 = "";
+    /* 1 = search string, 2 = replace with, 3 = argument order */
+    static char *saved1 = NULL; /* saved default[123] */
+    static char *saved2 = NULL;
+    static char *saved3 = NULL;
+    char *default1 = NULL; /* default values */
+    char *default2 = NULL;
+    char *default3 = NULL;
+    char *disp1 = NULL; /* default values translated to the display charset */
+    char *disp2 = NULL;
+    char *disp3 = NULL;
+    char *input1 = NULL; /* user input from the dialog */
+    char *input2 = NULL;
+    char *input3 = NULL;
     int replace_yes;
     int replace_continue;
     int treplace_prompt = 0;
-    int i = 0;
     long times_replaced = 0, last_search;
     int argord[NUM_REPL_ARGS];
 
     if (!edit) {
-	if (old1) {
-	    g_free (old1);
-	    old1 = 0;
-	}
-	if (old2) {
-	    g_free (old2);
-	    old2 = 0;
-	}
-	if (old3) {
-	    g_free (old3);
-	    old3 = 0;
-	}
+    	g_free (saved1), saved1 = NULL;
+    	g_free (saved2), saved2 = NULL;
+    	g_free (saved3), saved3 = NULL;
 	return;
     }
+
     last_search = edit->last_byte;
 
     edit->force |= REDRAW_COMPLETELY;
 
-    exp1 = old1 ? old1 : exp1;
-    exp2 = old2 ? old2 : exp2;
-    exp3 = old3 ? old3 : exp3;
+    default1 = g_strdup (saved1 ? saved1 : "");
+    default2 = g_strdup (saved2 ? saved2 : "");
+    default3 = g_strdup (saved3 ? saved3 : "");
 
     if (again) {
-	if (!old1 || !old2)
-	    return;
-	exp1 = g_strdup (old1);
-	exp2 = g_strdup (old2);
-	exp3 = g_strdup (old3);
+	if (saved1 == NULL || saved2 == NULL)
+	    goto cleanup;
     } else {
 	edit_push_action (edit, KEY_PRESS + edit->start_display);
 
-	convert_to_display (exp1);
-	convert_to_display (exp2);
-
-	edit_replace_dialog (edit, &exp1, &exp2, &exp3);
-
-	convert_from_input (exp1);
-	convert_from_input (exp2);
+	disp1 = g_strdup (default1);
+	disp2 = g_strdup (default2);
+	disp3 = g_strdup (default3);
+	convert_to_display (disp1);
+	convert_to_display (disp2);
+	convert_to_display (disp3);
+
+	edit_replace_dialog (edit, disp1, disp2, disp3, &input1, &input2, &input3);
+
+	convert_from_input (input1);
+	convert_from_input (input2);
+	convert_from_input (input3);
 
 	treplace_prompt = replace_prompt;
     }
 
-    if (!exp1 || !*exp1) {
+    if (input1 == NULL || *input1 == '\0') {
 	edit->force = REDRAW_COMPLETELY;
-	g_free (exp1);
-	g_free (exp2);
-	g_free (exp3);
-	return;
+	goto cleanup;
     }
-    g_free (old1);
-    g_free (old2);
-    g_free (old3);
-    old1 = g_strdup (exp1);
-    old2 = g_strdup (exp2);
-    old3 = g_strdup (exp3);
+
+    g_free (saved1);
+    g_free (saved2);
+    g_free (saved3);
+    saved1 = g_strdup (input1);
+    saved2 = g_strdup (input2);
+    saved3 = g_strdup (input3);
 
     {
-	char *s;
+	const char *s;
 	int ord;
-	while ((s = strchr (exp3, ' ')))
-	    memmove (s, s + 1, strlen (s));
-	s = exp3;
+	size_t i;
+
+	s = input3;
 	for (i = 0; i < NUM_REPL_ARGS; i++) {
-	    if (s != (char *) 1 && *s) {
+	    if (s != NULL && *s != '\0') {
 		ord = atoi (s);
 		if ((ord > 0) && (ord <= NUM_REPL_ARGS))
 		    argord[i] = ord - 1;
 		else
 		    argord[i] = i;
-		s = strchr (s, ',') + 1;
+		s = strchr (s, ',');
+		if (s != NULL)
+		    s++;
 	    } else
 		argord[i] = i;
 	}
@@ -1848,9 +1854,8 @@
 	int len = 0;
 	long new_start;
 	new_start =
-	    edit_find (edit->search_start, (unsigned char *) exp1, &len,
-		       last_search, (int (*)(void *, long)) edit_get_byte,
-		       (void *) edit, pmatch);
+	    edit_find (edit->search_start, (unsigned char *) input1, &len,
+		       last_search, edit_get_byte, (void *) edit, pmatch);
 	if (new_start == -3) {
 	    regexp_error (edit);
 	    break;
@@ -1859,6 +1864,8 @@
 	/*returns negative on not found or error in pattern */
 
 	if (edit->search_start >= 0) {
+	    int i;
+
 	    edit->found_start = edit->search_start;
 	    i = edit->found_len = len;
 
@@ -1882,7 +1889,7 @@
 		/*so that undo stops at each query */
 		edit_push_key_press (edit);
 
-		switch (edit_replace_prompt (edit, exp2,	/* and prompt 2/3 down */
+		switch (edit_replace_prompt (edit, input2,	/* and prompt 2/3 down */
 					     (edit->num_widget_columns -
 					      CONFIRM_DLG_WIDTH) / 2,
 					     edit->num_widget_lines * 2 /
@@ -1943,7 +1950,7 @@
 			    sargs[k - 1][0] = 0;
 		    }
 		    if (!ret)
-			ret = snprintf_p (repl_str, MAX_REPL_LEN + 2, exp2, PRINTF_ARGS);
+			ret = snprintf_p (repl_str, MAX_REPL_LEN + 2, input2, PRINTF_ARGS);
 		    if (ret >= 0) {
 			times_replaced++;
 			while (i--)
@@ -1961,8 +1968,8 @@
 		    times_replaced++;
 		    while (i--)
 			edit_delete (edit);
-		    while (exp2[++i])
-			edit_insert (edit, exp2[i]);
+		    while (input2[++i])
+			edit_insert (edit, input2[i]);
 		}
 		edit->found_len = i;
 	    }
@@ -1992,11 +1999,18 @@
 	}
     } while (replace_continue);
 
-    g_free (exp1);
-    g_free (exp2);
-    g_free (exp3);
     edit->force = REDRAW_COMPLETELY;
     edit_scroll_screen_over_cursor (edit);
+cleanup:
+    g_free (default1);
+    g_free (default2);
+    g_free (default3);
+    g_free (disp1);
+    g_free (disp2);
+    g_free (disp3);
+    g_free (input1);
+    g_free (input2);
+    g_free (input3);
 }
 
 
@@ -2049,7 +2063,7 @@
 		long p, q = 0;
 		for (;;) {
 		    p = edit_find (q, (unsigned char *) exp, &len, edit->last_byte,
-				   (int (*)(void *, long)) edit_get_byte, (void *) edit, 0);
+				   edit_get_byte, (void *) edit, 0);
 		    if (p < 0)
 			break;
 		    found++;
@@ -2076,7 +2090,7 @@
 		    edit->search_start++;
 
 		edit->search_start = edit_find (edit->search_start, (unsigned char *) exp, &len, edit->last_byte,
-		(int (*)(void *, long)) edit_get_byte, (void *) edit, 0);
+		                                edit_get_byte, (void *) edit, 0);
 
 		if (edit->search_start >= 0) {
 		    edit->found_start = edit->search_start;
@@ -2709,7 +2723,7 @@
 	start =
 	    edit_find (start - 1, (unsigned char *) match_expr, &len,
 		       edit->last_byte,
-		       (int (*)(void *, long)) edit_get_byte,
+		       edit_get_byte,
 		       (void *) edit, 0);
 
 	/* not matched */