xfs
[Top] [All Lists]

[PATCH v2] mkfs: better error with incorrect b/s value suffix usage

To: xfs@xxxxxxxxxxx
Subject: [PATCH v2] mkfs: better error with incorrect b/s value suffix usage
From: Jan Tulak <jtulak@xxxxxxxxxx>
Date: Thu, 9 Jun 2016 15:27:41 +0200
Cc: Jan Tulak <jtulak@xxxxxxxxxx>
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1465475195-19149-1-git-send-email-jtulak@xxxxxxxxxx>
References: <1465475195-19149-1-git-send-email-jtulak@xxxxxxxxxx>
If user writes a value using b or s suffix without explicitly stating the size
of blocks or sectors, mkfs ends with a not helpful error about the value being
too small. It happens because we read the physical geometry after all options
are parsed.

So, tell the user exactly what is wrong with the input.

Signed-off-by: Jan Tulak <jtulak@xxxxxxxxxx>
---

While adding entries into the mkfs input test, I found this issue, though it
may be only a documentation thing. When I give some option block/sector suffix
without specifying explicitly its size (for example, -l su=10b without -b
size=4096), I get an error that value 10b is too small.  Of course it is,
because, at the time, mkfs did not read physical geometry yet, so blocksize is
0. And 10*0 = 0.

I think that this is not something we need to change, but it should be better
documented. Maybe not manpage (where it can be overlooked if not written to
every option using the size and it might be that it already is somewhere down
there), but an error message should warn the user in case of using b or s
suffix incorrectly.

I'm open to suggestions for a better solution, though.

UPDATE:
Add { and } to fix a gcc warning about ambigious else branch.

Cheers,
Jan
---
 mkfs/xfs_mkfs.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index ed7800f..455bf11 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -3614,10 +3614,24 @@ cvtnum(
        if (sp[1] != '\0')
                return -1LL;
 
-       if (*sp == 'b')
-               return i * blksize;
-       if (*sp == 's')
-               return i * sectsize;
+       if (*sp == 'b') {
+               if (!blksize) {
+                       fprintf(stderr,
+_("Blocksize must be explicitly provided when using 'b' suffix.\n"));
+                       usage();
+               } else {
+                       return i * blksize;
+               }
+       }
+       if (*sp == 's') {
+               if (!sectsize) {
+                       fprintf(stderr,
+_("Sectorsize must be explicitly provided when using 's' suffix.\n"));
+                       usage();
+               } else {
+                       return i * sectsize;
+               }
+       }
 
        c = tolower(*sp);
        switch (c) {
-- 
2.5.5

<Prev in Thread] Current Thread [Next in Thread>