[parted-devel] [PATCH] Diagnose invalid command arguments.

[Jim Meyering]

This started because I objected to parted failing with no diagnostic
when given an invalid file system type:

    $ /sbin/parted -s $dev mklabel loop mkpartfs hfsplus 0 1.4
    WARNING: You are not superuser.  Watch out for permissions.
    [Exit 1]

With the changes below, it does this:

    $ ./parted -s $dev mklabel loop mkpartfs hfsplus 0 1.4
    ./parted: invalid token: hfsplus
    [Exit 1]

The following may look like a simple change, but looks are deceptive...
For example, if you try to diagnose via ped_exception_throw instead
of "error", you'll find that the mere fact of diagnosing the problem
introduces new ones because of how the exception handler manipulates the
global command line buffer containing the token we're complaining about.

But this isn't library code, so using error() is fine.

	Diagnose invalid command arguments.
	When a command argument doesn't match the expected candidate values,
	parted would silently exit (in script mode) or simply act as if that
	value and any following ones had not been specified (in interactive
	mode).  With this change, it complains about the "invalid token",
	and in script mode (where there hasn't been a prompt to give context)
	sometimes tells what type of token it was expecting.
	* parted/ui.c: Include "error.h".
	(command_line_get_word): If the user's "token" wasn't a good enough
	match, give a diagnostic.  In script mode, return NULL so that the
	callers can give additional information.
	* tests/t2000-mkfs.sh: New test for the above.
	* tests/t0000-basic.sh: Expect the new diagnostic when "msdos" is
	treated as an unrecognized first token after "mklabel".  This happens
	when trying to label a disk that already has a label.
	* tests/t1100-busy-label.sh: Likewise.

diff --git a/parted/ui.c b/parted/ui.c
index fb311e2..58e4563 100644
--- a/parted/ui.c
+++ b/parted/ui.c
@@ -32,6 +32,7 @@
 #include "command.h"
 #include "strlist.h"
 #include "ui.h"
+#include "error.h"

 #define N_(String) String
 #if ENABLE_NLS
@@ -866,11 +867,15 @@ command_line_get_word (const char* prompt, const char* def,
                                 return result;

                         result_node = str_list_match (possibilities, result);
+                        if (result_node == NULL)
+                                error (0, 0, _("invalid token: %s"), result);
                         free (result);
                         if (result_node)
                                 return str_list_convert_node (result_node);

                         command_line_flush ();
+                        if (opt_script_mode)
+                                return NULL;
                 }

                 command_line_prompt_words (prompt, def, possibilities,
diff --git a/tests/t0000-basic.sh b/tests/t0000-basic.sh
index f7a0c67..35bf6e6 100755
--- a/tests/t0000-basic.sh
+++ b/tests/t0000-basic.sh
@@ -85,6 +85,7 @@ fail=0
 cat <<EOF >> exp || fail=1
 Warning: The existing disk label on DEVICE will be destroyed and all\
  data on this disk will be lost. Do you want to continue?
+parted: invalid token: msdos
 Yes/No? y
 New disk label type?  [msdos]?
 EOF
diff --git a/tests/t1100-busy-label.sh b/tests/t1100-busy-label.sh
index b1f1659..d571ae1 100755
--- a/tests/t1100-busy-label.sh
+++ b/tests/t1100-busy-label.sh
@@ -68,6 +68,7 @@ test_expect_failure \
 fail=0
 cat <<EOF > exp || fail=1
 Warning: Partition(s) on $dev are being used.
+parted: invalid token: msdos
 Ignore/Cancel? c
 EOF
 test_expect_success 'create expected output file' 'test $fail = 0'
diff --git a/tests/t2000-mkfs.sh b/tests/t2000-mkfs.sh
index 79d327e..fb39815 100755
--- a/tests/t2000-mkfs.sh
+++ b/tests/t2000-mkfs.sh
@@ -33,7 +33,7 @@ test_expect_success \
 test_expect_success 'expect no output' '$compare out /dev/null'

 test_expect_success \
-    'create an partition' \
+    'create a partition' \
     'parted -s $dev mkpart primary 1 40 > out 2>&1'

 test_expect_success \
@@ -80,4 +80,25 @@ test_expect_success \
        "data on the partition will be lost. Do you want to continue?" > exp &&
      $compare out2 exp'

+#############################################################
+# Ensure that an invalid file system type elicits a diagnostic.
+# Before parted 1.8.8, this would fail silently.
+
+dev=loop-file
+
+test_expect_success \
+    "setup: create and label a device" \
+    'dd if=/dev/zero of=$dev bs=1M count=1 2>/dev/null &&
+     parted -s $dev mklabel msdos'
+
+test_expect_failure \
+    'try to create a file system with invalid type name' \
+    'parted -s $dev mkpartfs primary bogus 1 1 >out 2>&1'
+
+test_expect_success \
+    'check for expected diagnostic' \
+    '{ echo "parted: invalid token: bogus"
+       echo "Error: Expecting a file system type."; } > exp &&
+     $compare out exp'
+
 test_done