[BusyBox] regex updates

[Matt Kraai]

Howdy,

On Tue, Jul 11, 2000 at 10:57:19AM -0600, Mark Whitley wrote:
> Thanks, Matt, this looks great. I have applied the patch with the following
> modifications:
> 
>  - I still want to call exit_sed() in sed.c rather than fatalError because
>    exit_sed() will in turn call destroy_cmd_strs() to free the dynamically
>    allocated sed_cmds array.

I'm a little confused why you go through so much trouble.  First, why
does the memory need to be freed, since it will be anyway on process
exit.  And if it really does need to be freed, why not use the standard
atexit()?  This would allow exit_sed to be axed.  I have attached a
patch to do this.

>  - I changed bb_regcomp() to return the int value returned from libc regcomp()
>    (rather than it being a void function that calls exit(1) on failure) so
>    that calling apps can do their own error handling. (In the case of sed, for
>    example, I want to call exit_sed() if bb_regcomp() fails, for reasons
>    mentioned already. BTW: I think bb_regcomp() is a great function name and
>    the bb_* prefix is a convention we should follow more often, esp in
>    utility.c.

For me, bb_regcomp exists only to consolidate the error handling code.
I'd rather it not return at all in case of error, but I can see that if
you really do need to free all extra memory then it must.
 
>  - Thanks for adding the feature to do case-insensitive matching in s///
>    expressions. I have changed the s/ flag from 'i' to 'I' so as to A) bring
>    our sed into closer compliance with GNU sed and B) remove any confusion /
>    conflict with the (i)nsert command found in GNU/POSIX sed.
> 
>  - Some other very small / cosmetic cleanups & mods: more cruft removal from
>    sed.c and putting "bb_regcomp" in the format string passed to errMsg() in
>    the bb_regcomp() function.
> 
> BTW, I noticed you cleaned up some of my cruft from when I was developing sed
> off-line. Thanks for that, too.

These all sound good.  Let me know what you think about the above.

Matt
-------------- next part --------------
Index: sed.c
===================================================================
RCS file: /var/cvs/busybox/sed.c,v
retrieving revision 1.17
diff -u -r1.17 sed.c
--- sed.c	2000/07/11 16:53:56	1.17
+++ sed.c	2000/07/11 18:50:15
@@ -126,14 +126,6 @@
 	sed_cmds = NULL;
 }
 
-static void exit_sed(int retcode, const char *message)
-{
-	destroy_cmd_strs();
-	if (message)
-		fputs(message, stderr);
-	exit(retcode);
-}
-
 /*
  * trim_str - trims leading and trailing space from a string
  * 
@@ -204,12 +196,12 @@
 	else if (my_str[idx] == '/') {
 		idx = index_of_next_unescaped_slash(idx, my_str);
 		if (idx == -1)
-			exit_sed(1, "sed: unterminated match expression\n");
+			fatalError("sed: unterminated match expression\n");
 		my_str[idx] = '\0';
 		*regex = (regex_t *)xmalloc(sizeof(regex_t));
 		if (bb_regcomp(*regex, my_str+1, REG_NEWLINE) != 0) {
 			free(my_str);
-			exit_sed(1, NULL);
+			exit(1);
 		}
 	}
 	else {
@@ -251,9 +243,9 @@
 
 	/* last part (mandatory) will be a command */
 	if (cmdstr[idx] == '\0')
-		exit_sed(1, "sed: missing command\n");
+		fatalError("sed: missing command\n");
 	if (!strchr("pds", cmdstr[idx])) /* <-- XXX add new commands here */
-		exit_sed(1, "sed: invalid command\n");
+		fatalError("sed: invalid command\n");
 	sed_cmd->cmd = cmdstr[idx];
 	/* special-case handling for 's' */
 	if (sed_cmd->cmd == 's') {
@@ -267,20 +259,20 @@
 
 		/* verify that we have an 's' followed by a 'slash' */
 		if (cmdstr[++idx] != '/')
-			exit_sed(1, "sed: bad format in substitution expression\n");
+			fatalError("sed: bad format in substitution expression\n");
 
 		/* save the match string */
 		oldidx = idx+1;
 		idx = index_of_next_unescaped_slash(idx, cmdstr);
 		if (idx == -1)
-			exit_sed(1, "sed: bad format in substitution expression\n");
+			fatalError("sed: bad format in substitution expression\n");
 		match = strdup_substr(cmdstr, oldidx, idx);
 
 		/* save the replacement string */
 		oldidx = idx+1;
 		idx = index_of_next_unescaped_slash(idx, cmdstr);
 		if (idx == -1)
-			exit_sed(1, "sed: bad format in substitution expression\n");
+			fatalError("sed: bad format in substitution expression\n");
 		sed_cmd->replace = strdup_substr(cmdstr, oldidx, idx);
 
 		/* process the flags */
@@ -293,7 +285,7 @@
 				cflags |= REG_ICASE;
 				break;
 			default:
-				exit_sed(1, "sed: bad option in substitution expression\n");
+				fatalError("sed: bad option in substitution expression\n");
 			}
 		}
 			
@@ -301,7 +293,7 @@
 		sed_cmd->sub_match = (regex_t *)xmalloc(sizeof(regex_t));
 		if (bb_regcomp(sed_cmd->sub_match, match, cflags) != 0) {
 			free(match);
-			exit_sed(1, NULL);
+			exit(1);
 		}
 		free(match);
 	}
@@ -333,7 +325,7 @@
 
 	cmdfile = fopen(filename, "r");
 	if (cmdfile == NULL)
-		exit_sed(1, strerror(errno));
+		fatalError(strerror(errno));
 
 	while ((line = get_line_from_file(cmdfile)) != NULL) {
 		line[strlen(line)-1] = 0; /* eat newline */
@@ -464,10 +456,16 @@
 {
 	int opt;
 
-    /* do special-case option parsing */
+	/* do special-case option parsing */
 	if (argv[1] && (strcmp(argv[1], "--help") == 0))
 		usage(sed_usage);
 
+	/* destroy command strings on exit */
+	if (atexit(destroy_cmd_strs) == -1) {
+		perror("sed");
+		exit(1);
+	}
+
 	/* do normal option parsing */
 	while ((opt = getopt(argc, argv, "Vhne:f:")) > 0) {
 		switch (opt) {
@@ -522,8 +520,5 @@
 		}
 	}
 	
-	exit_sed(0, NULL);
-
-	/* not reached */
 	return 0;
 }
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 232 bytes
Desc: not available
Url : http://lists.busybox.net/pipermail/busybox/attachments/20000711/1de87b24/attachment.pgp