[gPXE-devel] Minor Scripting Improvements

[Shao Miller]

Michael Brown wrote:
>
> Looking at the patch itself...
>
> Could this be rearranged to avoid goto?  We generally use goto only for
> structured error clean-up.
>
> Also, using -ENOTSUP as a magic value meaning "loop" seems kind of icky. 
> There's a similarity between "exit" and "loopif" here; both commands 
> need to
> set temporary state that affects the "move to next line of script" logic. 
> Maybe have "loopif" set a flag as well; still fairly icky but at least 
> it's
> icky in the same way that "exit" currently is.
>

Or perhaps this patch, which costs 82 bytes uncompressed, and uses a 
separate looping variable.

- Shao Miller
-----

diff --git a/src/hci/shell.c b/src/hci/shell.c
index 5bedbdc..825dac3 100644
--- a/src/hci/shell.c
+++ b/src/hci/shell.c
@@ -35,13 +35,13 @@ FILE_LICENCE ( GPL2_OR_LATER );
 static const char shell_prompt[] = "gPXE> ";
 
 /** Flag set in order to exit shell */
-static int exit_flag = 0;
+int shell_exit_flag = 0;
 
 /** "exit" command body */
 static int exit_exec ( int argc, char **argv __unused ) {
 
     if ( argc == 1 ) {
-        exit_flag = 1;
+        shell_exit_flag = 1;
     } else {
         printf ( "Usage: exit\n"
              "Exits the command shell\n" );
@@ -91,8 +91,8 @@ struct command help_command __command = {
 void shell ( void ) {
     char *line;
 
-    exit_flag = 0;
-    while ( ! exit_flag ) {
+    shell_exit_flag = 0;
+    while ( ! shell_exit_flag ) {
         line = readline ( shell_prompt );
         if ( line ) {
             system ( line );
diff --git a/src/image/script.c b/src/image/script.c
old mode 100644
new mode 100755
index 0835ecb..fd1d4a2
--- a/src/image/script.c
+++ b/src/image/script.c
@@ -29,10 +29,15 @@ FILE_LICENCE ( GPL2_OR_LATER );
 #include <stdlib.h>
 #include <ctype.h>
 #include <errno.h>
+#include <gpxe/command.h>
 #include <gpxe/image.h>
+#include <gpxe/shell.h>
 
 struct image_type script_image_type __image_type ( PROBE_NORMAL );
 
+/** Flag cleared in order to loop the current script */
+static int keep_offset = 1;
+
 /**
  * Execute script
  *
@@ -51,7 +56,10 @@ static int script_exec ( struct image *image ) {
     unregister_image ( image );
 
     while ( offset < image->len ) {
-   
+        /* Reset the exit and loop flags */
+        shell_exit_flag = 0;
+        keep_offset = 1;
+
         /* Find length of next line, excluding any terminating '\n' */
         eol = memchr_user ( image->data, offset, '\n',
                     ( image->len - offset ) );
@@ -66,15 +74,19 @@ static int script_exec ( struct image *image ) {
             copy_from_user ( cmdbuf, image->data, offset, len );
             cmdbuf[len] = '\0';
             DBG ( "$ %s\n", cmdbuf );
-            if ( ( rc = system ( cmdbuf ) ) != 0 ) {
-                DBG ( "Command \"%s\" failed: %s\n",
+            rc = system ( cmdbuf );
+            rc |= shell_exit_flag;
+            if ( rc ) {
+                DBG ( "Command \"%s\" failed or exited: %s\n",
                       cmdbuf, strerror ( rc ) );
                 goto done;
             }
         }
-       
+
         /* Move to next line */
         offset += ( len + 1 );
+        /* "loopif" command might want us to reset the offset */
+        offset *= keep_offset;
     }
 
     rc = 0;
@@ -124,3 +136,16 @@ struct image_type script_image_type __image_type ( 
PROBE_NORMAL ) = {
     .load = script_load,
     .exec = script_exec,
 };
+
+/** "loopif" command body */
+static int loopif_exec ( int argc, char **argv ) {
+    if ( argc > 1 && strtoul ( argv[1], NULL, 0 ) )
+        keep_offset = 0;
+    return 0;
+}
+
+/** "loopif" command definition */
+struct command loopif_command __command = {
+    .name = "loopif",
+    .exec = loopif_exec,
+};
diff --git a/src/include/gpxe/shell.h b/src/include/gpxe/shell.h
index a65a344..148ea83 100644
--- a/src/include/gpxe/shell.h
+++ b/src/include/gpxe/shell.h
@@ -9,6 +9,7 @@
 
 FILE_LICENCE ( GPL2_OR_LATER );
 
+extern int shell_exit_flag;
 extern void shell ( void );
 
 #endif /* _GPXE_SHELL_H */

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://etherboot.org/pipermail/gpxe-devel/attachments/20100305/66fafb62/attachment-0001.html 
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: exit_loop3.patch
Url: http://etherboot.org/pipermail/gpxe-devel/attachments/20100305/66fafb62/attachment-0001.cc