[PATCH 1/2] blkid: use symbolic exit code

[PATCH 1/2] blkid: use symbolic exit code

[Petr Uzel]

Signed-off-by: Petr Uzel <petr.uzel@suse.cz>
---
 misc-utils/blkid.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/misc-utils/blkid.c b/misc-utils/blkid.c
index 8007840..4f7050f 100644
--- a/misc-utils/blkid.c
+++ b/misc-utils/blkid.c
@@ -720,7 +720,7 @@ int main(int argc, char **argv)
 
 			while (blkid_superblocks_get_name(idx++, &name, NULL) == 0)
 				printf("%s\n", name);
-			exit(0);
+			exit(EXIT_SUCCESS);
 		}
 		case 'o':
 			if (!strcmp(optarg, "value"))
-- 
1.7.7

[PATCH 2/2] blkid: introduce symbolic names for different blkid exit codes

[Petr Uzel]

Signed-off-by: Petr Uzel <petr.uzel@suse.cz>
---
 misc-utils/blkid.c |   32 ++++++++++++++++++--------------
 1 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/misc-utils/blkid.c b/misc-utils/blkid.c
index 4f7050f..52b2a4b 100644
--- a/misc-utils/blkid.c
+++ b/misc-utils/blkid.c
@@ -34,11 +34,15 @@ extern int optind;
 #define LOWPROBE_TOPOLOGY	(1 << 1)
 #define LOWPROBE_SUPERBLOCKS	(1 << 2)
 
+#define BLKID_EXIT_NOTFOUND	2	/* token or device not found */
+#define BLKID_EXIT_OTHER	4	/* bad usage or other error */
+#define BLKID_EXIT_AMBIVAL	8	/* ambivalent low-level probing detected */
+
 #include <blkid.h>
 
 #include "ismounted.h"
 
-#define STRTOXX_EXIT_CODE	4		/* strtoxx_or_err() */
+#define STRTOXX_EXIT_CODE	BLKID_EXIT_OTHER	/* strtoxx_or_err() */
 #include "strutils.h"
 #include "closestream.h"
 #include "ttyutils.h"
@@ -484,7 +488,7 @@ static int lowprobe_device(blkid_probe pr, const char *devname,
 	fd = open(devname, O_RDONLY);
 	if (fd < 0) {
 		fprintf(stderr, "error: %s: %m\n", devname);
-		return 2;
+		return BLKID_EXIT_NOTFOUND;
 	}
 	if (blkid_probe_set_device(pr, fd, offset, size))
 		goto done;
@@ -551,9 +555,9 @@ done:
 	close(fd);
 
 	if (rc == -2)
-		return 8;	/* ambivalent probing result */
+		return BLKID_EXIT_AMBIVAL;	/* ambivalent probing result */
 	if (!nvals)
-		return 2;	/* nothing detected */
+		return BLKID_EXIT_NOTFOUND;	/* nothing detected */
 
 	return 0;		/* success */
 }
@@ -591,7 +595,7 @@ err:
 	*flag = 0;
 	fprintf(stderr, "unknown kerword in -u <list> argument: '%s'\n",
 			word ? word : list);
-	exit(4);
+	exit(BLKID_EXIT_OTHER);
 }
 
 /* converts comma separated list to types[] */
@@ -633,7 +637,7 @@ err_mem:
 err:
 	*flag = 0;
 	free(res);
-	exit(4);
+	exit(BLKID_EXIT_OTHER);
 }
 
 static void free_types_list(char *list[])
@@ -659,7 +663,7 @@ int main(int argc, char **argv)
 	int fltr_flag = BLKID_FLTR_ONLYIN;
 	unsigned int numdev = 0, numtag = 0;
 	int version = 0;
-	int err = 4;
+	int err = BLKID_EXIT_OTHER;
 	unsigned int i;
 	int output_format = 0;
 	int lookup = 0, gc = 0, lowprobe = 0, eval = 0;
@@ -688,14 +692,14 @@ int main(int argc, char **argv)
 		case 'n':
 			if (fltr_usage) {
 				fprintf(stderr, "error: -u and -n options are mutually exclusive\n");
-				exit(4);
+				exit(BLKID_EXIT_OTHER);
 			}
 			fltr_type = list_to_types(optarg, &fltr_flag);
 			break;
 		case 'u':
 			if (fltr_type) {
 				fprintf(stderr, "error: -u and -n options are mutually exclusive\n");
-				exit(4);
+				exit(BLKID_EXIT_OTHER);
 			}
 			fltr_usage = list_to_usage(optarg, &fltr_flag);
 			break;
@@ -739,7 +743,7 @@ int main(int argc, char **argv)
 				fprintf(stderr, "Invalid output format %s. "
 					"Choose from value,\n\t"
 					"device, list, udev or full\n", optarg);
-				exit(4);
+				exit(BLKID_EXIT_OTHER);
 			}
 			break;
 		case 'O':
@@ -818,13 +822,13 @@ int main(int argc, char **argv)
 		err = 0;
 		goto exit;
 	}
-	err = 2;
+	err = BLKID_EXIT_NOTFOUND;
 
 	if (eval == 0 && (output_format & OUTPUT_PRETTY_LIST)) {
 		if (lowprobe) {
 			fprintf(stderr, "The low-level probing mode does not "
 					"support 'list' output format\n");
-			exit(4);
+			exit(BLKID_EXIT_OTHER);
 		}
 		pretty_print_dev(NULL);
 	}
@@ -838,7 +842,7 @@ int main(int argc, char **argv)
 		if (!numdev) {
 			fprintf(stderr, "The low-level probing mode "
 					"requires a device\n");
-			exit(4);
+			exit(BLKID_EXIT_OTHER);
 		}
 
 		/* automatically enable 'export' format for I/O Limits */
@@ -888,7 +892,7 @@ int main(int argc, char **argv)
 		if (!search_type) {
 			fprintf(stderr, "The lookup option requires a "
 				"search type specified using -t\n");
-			exit(4);
+			exit(BLKID_EXIT_OTHER);
 		}
 		/* Load any additional devices not in the cache */
 		for (i = 0; i < numdev; i++)
-- 
1.7.7

RE: [PATCH 2/2] blkid: introduce symbolic names for different blkid exit codes

[Voelker, Bernhard]

Petr Uzel wrote:

> Subject: [PATCH 2/2] blkid: introduce symbolic names for different blkid exit codes
> 
> 
> Signed-off-by: Petr Uzel <petr.uzel@suse.cz>
> ---
>  misc-utils/blkid.c |   32 ++++++++++++++++++--------------
>  1 files changed, 18 insertions(+), 14 deletions(-)
> 
> diff --git a/misc-utils/blkid.c b/misc-utils/blkid.c
> index 4f7050f..52b2a4b 100644
> --- a/misc-utils/blkid.c
> +++ b/misc-utils/blkid.c
> @@ -34,11 +34,15 @@ extern int optind;
>  #define LOWPROBE_TOPOLOGY	(1 << 1)
>  #define LOWPROBE_SUPERBLOCKS	(1 << 2)
>  
> +#define BLKID_EXIT_NOTFOUND	2	/* token or device not found */
> +#define BLKID_EXIT_OTHER	4	/* bad usage or other error */
> +#define BLKID_EXIT_AMBIVAL	8	/* ambivalent low-level probing detected */
> +
>  #include <blkid.h>
>  
>  #include "ismounted.h"
>  
> -#define STRTOXX_EXIT_CODE	4		/* strtoxx_or_err() */
> +#define STRTOXX_EXIT_CODE	BLKID_EXIT_OTHER	/* strtoxx_or_err() */

STRTOXX_EXIT_CODE is not used in blkid.c. Remove it?
... or if it used in strutils.h, then this is not a nice interface.

>  #include "strutils.h"
>  #include "closestream.h"
>  #include "ttyutils.h"
> @@ -484,7 +488,7 @@ static int lowprobe_device(blkid_probe pr, const char *devname,
>  	fd = open(devname, O_RDONLY);
>  	if (fd < 0) {
>  		fprintf(stderr, "error: %s: %m\n", devname);
> -		return 2;
> +		return BLKID_EXIT_NOTFOUND;
>  	}
>  	if (blkid_probe_set_device(pr, fd, offset, size))
>  		goto done;
> @@ -551,9 +555,9 @@ done:
>  	close(fd);
>  
>  	if (rc == -2)
> -		return 8;	/* ambivalent probing result */
> +		return BLKID_EXIT_AMBIVAL;	/* ambivalent probing result */
>  	if (!nvals)
> -		return 2;	/* nothing detected */
> +		return BLKID_EXIT_NOTFOUND;	/* nothing detected */
>  
>  	return 0;		/* success */
>  }

These are not exit() codes, but return codes. That value doesn't seem
to be of much interest anyway, because it's used only here in main()
in a for-loop, forgetting the return value of the previous iteration:

               for (i = 0; i < numdev; i++)
                        err = lowprobe_device(pr, devices[i], lowprobe, show,
                                        output_format,
                                        (blkid_loff_t) offset,
                                        (blkid_loff_t) size);

Have a nice day,
Berny

Re: [PATCH 2/2] blkid: introduce symbolic names for different blkid exit codes

[Petr Uzel]

[-- Attachment #1: Type: text/plain, Size: 2400 bytes --]

On Wed, May 16, 2012 at 10:52:40AM +0200, Voelker, Bernhard wrote:
> Petr Uzel wrote:
> 
> >  #include "ismounted.h"
> >  
> > -#define STRTOXX_EXIT_CODE	4		/* strtoxx_or_err() */
> > +#define STRTOXX_EXIT_CODE	BLKID_EXIT_OTHER	/* strtoxx_or_err() */
> 
> STRTOXX_EXIT_CODE is not used in blkid.c. Remove it?
> ... or if it used in strutils.h, then this is not a nice interface.

Yep, it is used in strutils.h.

> 
> >  #include "strutils.h"
> >  #include "closestream.h"
> >  #include "ttyutils.h"
> > @@ -484,7 +488,7 @@ static int lowprobe_device(blkid_probe pr, const char *devname,
> >  	fd = open(devname, O_RDONLY);
> >  	if (fd < 0) {
> >  		fprintf(stderr, "error: %s: %m\n", devname);
> > -		return 2;
> > +		return BLKID_EXIT_NOTFOUND;
> >  	}
> >  	if (blkid_probe_set_device(pr, fd, offset, size))
> >  		goto done;
> > @@ -551,9 +555,9 @@ done:
> >  	close(fd);
> >  
> >  	if (rc == -2)
> > -		return 8;	/* ambivalent probing result */
> > +		return BLKID_EXIT_AMBIVAL;	/* ambivalent probing result */
> >  	if (!nvals)
> > -		return 2;	/* nothing detected */
> > +		return BLKID_EXIT_NOTFOUND;	/* nothing detected */
> >  
> >  	return 0;		/* success */
> >  }
> 
> These are not exit() codes, but return codes. 

Strictly speaking, yes. But the idea is that those codes are (or
rather should be) propagated as exit value anyways...

> That value doesn't seem
> to be of much interest anyway, because it's used only here in main()
> in a for-loop, forgetting the return value of the previous iteration:
> 
>                for (i = 0; i < numdev; i++)
>                         err = lowprobe_device(pr, devices[i], lowprobe, show,
>                                         output_format,
>                                         (blkid_loff_t) offset,
>                                         (blkid_loff_t) size);

Hmm, the return value depends only on the last argument (device). I
don't think this is intentional. Karel?

        # blkid -p /dev/sda /dev/sdb ; echo $?
        /dev/sda: PTTYPE="gpt" 
        error: /dev/sdb: No such file or directory
        2
        
        # blkid -p /dev/sdb /dev/sda ; echo $?
        error: /dev/sdb: No such file or directory
        /dev/sda: PTTYPE="gpt" 
        0


> 
> Have a nice day,
> Berny
 
Thanks, 

Petr

-- 
Petr Uzel
IRC: ptr_uzl @ freenode

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH 2/2] blkid: introduce symbolic names for different blkid exit codes

[Karel Zak]

On Wed, May 16, 2012 at 11:24:09AM +0200, Petr Uzel wrote:
> On Wed, May 16, 2012 at 10:52:40AM +0200, Voelker, Bernhard wrote:
> > Petr Uzel wrote:
> > 
> > >  #include "ismounted.h"
> > >  
> > > -#define STRTOXX_EXIT_CODE	4		/* strtoxx_or_err() */
> > > +#define STRTOXX_EXIT_CODE	BLKID_EXIT_OTHER	/* strtoxx_or_err() */
> > 
> > STRTOXX_EXIT_CODE is not used in blkid.c. Remove it?
> > ... or if it used in strutils.h, then this is not a nice interface.
> 
> Yep, it is used in strutils.h.

 Fortunately, STRTOXX_EXIT_CODE is rarely used (default is EXIT_FAILURE).

> > >  	if (rc == -2)
> > > -		return 8;	/* ambivalent probing result */
> > > +		return BLKID_EXIT_AMBIVAL;	/* ambivalent probing result */
> > >  	if (!nvals)
> > > -		return 2;	/* nothing detected */
> > > +		return BLKID_EXIT_NOTFOUND;	/* nothing detected */
> > >  
> > >  	return 0;		/* success */
> > >  }
> > 
> > These are not exit() codes, but return codes. 
> 
> Strictly speaking, yes. But the idea is that those codes are (or
> rather should be) propagated as exit value anyways...
> 
> > That value doesn't seem
> > to be of much interest anyway, because it's used only here in main()
> > in a for-loop, forgetting the return value of the previous iteration:
> > 
> >                for (i = 0; i < numdev; i++)
> >                         err = lowprobe_device(pr, devices[i], lowprobe, show,
> >                                         output_format,
> >                                         (blkid_loff_t) offset,
> >                                         (blkid_loff_t) size);
> 
> Hmm, the return value depends only on the last argument (device). I
> don't think this is intentional. Karel?
> 
>         # blkid -p /dev/sda /dev/sdb ; echo $?
>         /dev/sda: PTTYPE="gpt" 
>         error: /dev/sdb: No such file or directory
>         2
>         
>         # blkid -p /dev/sdb /dev/sda ; echo $?
>         error: /dev/sdb: No such file or directory
>         /dev/sda: PTTYPE="gpt" 
>         0

 It would be better to break the loop and exit after first error.

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

RE: [PATCH 2/2] blkid: introduce symbolic names for different blkid exit codes

[Voelker, Bernhard]

Karel Zak wrote:

> On Wed, May 16, 2012 at 11:24:09AM +0200, Petr Uzel wrote:
> > Hmm, the return value depends only on the last argument (device). I
> > don't think this is intentional. Karel?
> > 
> >         # blkid -p /dev/sda /dev/sdb ; echo $?
> >         /dev/sda: PTTYPE="gpt" 
> >         error: /dev/sdb: No such file or directory
> >         2
> >         
> >         # blkid -p /dev/sdb /dev/sda ; echo $?
> >         error: /dev/sdb: No such file or directory
> >         /dev/sda: PTTYPE="gpt" 
> >         0
> 
>  It would be better to break the loop and exit after first error.

Then we can already exit() from lowprobe_device(),
and thus BLKID_EXIT_* will have become pure "exit codes" ... ;-)

Have a nice day,
Berny

[PATCH] blkid: stop device probing if error is detected

[Petr Uzel]

blkid -p dev1 dev2 [...] now stops upon first error detected and
returns correct value. Previously, if error was detected with dev1,
it continued with dev2 anyways and so the return value corresponded
to the last device only.

Reported-by: Bernhard Voelker <bernhard.voelker@siemens-enterprise.com>
Signed-off-by: Petr Uzel <petr.uzel@suse.cz>
---
 misc-utils/blkid.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/misc-utils/blkid.c b/misc-utils/blkid.c
index 52b2a4b..25e6c00 100644
--- a/misc-utils/blkid.c
+++ b/misc-utils/blkid.c
@@ -868,11 +868,14 @@ int main(int argc, char **argv)
 				goto exit;
 		}
 
-		for (i = 0; i < numdev; i++)
+		for (i = 0; i < numdev; i++) {
 			err = lowprobe_device(pr, devices[i], lowprobe, show,
 					output_format,
 					(blkid_loff_t) offset,
 					(blkid_loff_t) size);
+			if (err)
+				break;
+		}
 		blkid_free_probe(pr);
 	} else if (eval) {
 		/*
-- 
1.7.7

Re: [PATCH] blkid: stop device probing if error is detected

[Karel Zak]

On Thu, May 17, 2012 at 09:37:25AM +0200, Petr Uzel wrote:
>  misc-utils/blkid.c |    5 ++++-
>  1 files changed, 4 insertions(+), 1 deletions(-)

 Applied, thanks.

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: [PATCH 2/2] blkid: introduce symbolic names for different blkid exit codes

[Karel Zak]

On Wed, May 16, 2012 at 10:38:32AM +0200, Petr Uzel wrote:
>  misc-utils/blkid.c |   32 ++++++++++++++++++--------------
>  1 files changed, 18 insertions(+), 14 deletions(-)

Applied, thanks.

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: [PATCH 1/2] blkid: use symbolic exit code

[Karel Zak]

On Wed, May 16, 2012 at 10:38:31AM +0200, Petr Uzel wrote:
>  misc-utils/blkid.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)

 Applied, thanks.

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com