Hello Michael Prokop!

Thanks for your bug report and sorry for the late answer.

On Thu, Dec 11, 2014 at 05:55:18PM +0100, Michael Prokop wrote:
[... snip blkid from util-linux 2.20.1-5.6 finds no tokens ...]
> 
> Now with with util-linux version 2.25.2-* we get instead:
[...]
> # blkid /dev/sda1
> /dev/sda1: PARTUUID="000e1222-01"

PARTUUID seems to have been introduced as a new tag in v2.22
(see commit fc387ee14c6b8672761ae5e67ff639b5cae8f27c).

> 
> This behaviour change caused a regression in grml-debootstrap for
> its filesystem detection because it relies on the exit code.
> 
> The manpage of blkid says:
> 
> ,---- [ man blkid ]
> | RETURN CODE
> |        If the specified token was found, or if any tags were shown from 
> (specified) devices, 0 is returned.
> |        If the specified token was not found, or no (specified) devices 
> could be identified, an exit code of 2 is returned.

The "the specified token" part of the above (second) sentence doesn't
seem to be true for either the old (wheezy) version of util-linux, or
the new (jessie) version.

# blkid -v
blkid from util-linux 2.25.2  (libblkid 2.25.0, 24-Oct-2014)
# blkid -s FOOBAR /dev/sda1 ; echo $?
0
# blkid /dev/sda1
/dev/sda1: LABEL="EFI" UUID="67E3-17ED" TYPE="vfat" PARTLABEL="EFI
System Partition" PARTUUID="583492ba-42c9-4af1-aeaa-aa93ff462134"

# blkid -v
blkid from util-linux 2.20.1 (libblkid 2.20.0, 19-Oct-2011)
# blkid -s FOOBAR /dev/sda1 ; echo $?
0
# blkid /dev/sda1
/dev/sda1: LABEL="EFI" UUID="67E3-17ED" TYPE="vfat" 

> |        For usage or other errors, an exit code of 4 is returned.
> |        If the ambivalent low-level probing result was detected, an exit 
> code of 8 is returned.
> `----
> 
> I think either the man page is wrong/misleading and/or the exit code
> of "blkid -s TYPE -o value ..." is wrong if nothing is reported?

Possibly so. See above. This doesn't seem like a new issue however.

Your bug report in my view consists of two separate parts.
1. The part in the beginning caused by the corner case of what
happens when a new token gets added.
2. The manpage/implementation inconsistencies.

I guess both of them in combination is the full cause of the
grml-debootstrap problem.

I'm not sure I'd dare change the behaviour but keeping the behaviour
more or less makes the exit code 2 useless. I'll raise the
issue on the upstream mailing list to see if fixing the manpage to
describe the actual behaviour of both/all versions or changing the code
to behave like described in the manpage is preferred.
(I've attached a patch which should implement my interpretation of the
described behaviour. I will personally not take on any attempts at
improving manpage wordings as I'm not a native english speaker.)

>From Debian perspective we could also consider if we should add a Breaks
on the old grml-debootstrap version, similar to what is suggested for
dealing with brokenness in old versions of live-tools in #773354.
(Not sure how much I like dumping buckets of Breaks on util-linux though,
specially in cases like #773354 where it's seems util-linux is only one
of the triggers of update-initramfs which is what triggers the
live-tools bug.)

Regards,
Andreas Henriksson
>From b89ff75640d97c9084ada21212bdd38a30da1456 Mon Sep 17 00:00:00 2001
From: Andreas Henriksson <[email protected]>
Date: Thu, 1 Jan 2015 21:55:54 +0100
Subject: [PATCH] blkid: exit 2 when no specified tags match

,---- [ man blkid ]
| RETURN CODE
|        If the specified token was found, or if any tags were shown from (specified) devices, 0 is returned.
|        If the specified token was not found, or no (specified) devices could be identified, an exit code of 2 is returned.
|        For usage or other errors, an exit code of 4 is returned.
|        If the ambivalent low-level probing result was detected, an exit code of 8 is returned.
`----

The code doesn't seem to work like described in the (first part of the)
second sentence. The exit code is only 2 if *no* tokens was identified,
0 if tokens was found but none matched the filter.

This changes the code so that the exit code is 2 if the *specified* tag
is not found, as described in the manpage.

Reported-by: Michael Prokop <[email protected]>
Addresses: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=772846
---
 misc-utils/blkid.c | 29 +++++++++++++++++++----------
 1 file changed, 19 insertions(+), 10 deletions(-)


WARNING: This is a behaviour change. Maybe it's safer to just change
the manpage to document the actual behaviour instead of changing it.
Please review carefully and decide if this patch is safe (and correct
since it's only been very lightly tested).

Further work: The mixed usage of 'tokens' and 'tag(s)' in the manpage
confuses me and unless I'm mistaken is sometimes also wrong. Eg. you
specify a tag with "-s <tag>", there's no way "specified token" is
possible, et.al. It would be welcome if anyone wants to review and
improve the manpage to be easier to understand.


diff --git a/misc-utils/blkid.c b/misc-utils/blkid.c
index 1bd8646..e605df2 100644
--- a/misc-utils/blkid.c
+++ b/misc-utils/blkid.c
@@ -320,7 +320,7 @@ static void print_value(int output, int num, const char *devname,
 	}
 }
 
-static void print_tags(blkid_dev dev, char *show[], int output)
+static int print_tags(blkid_dev dev, char *show[], int output)
 {
 	blkid_tag_iterate	iter;
 	const char		*type, *value, *devname;
@@ -328,18 +328,18 @@ static void print_tags(blkid_dev dev, char *show[], int output)
 	static int		first = 1;
 
 	if (!dev)
-		return;
+		return 0;
 
 	if (output & OUTPUT_PRETTY_LIST) {
 		pretty_print_dev(dev);
-		return;
+		return 0;
 	}
 
 	devname = blkid_dev_devname(dev);
 
 	if (output & OUTPUT_DEVICE_ONLY) {
 		printf("%s\n", devname);
-		return;
+		return 0;
 	}
 
 	iter = blkid_tag_iterate_begin(dev);
@@ -362,6 +362,10 @@ static void print_tags(blkid_dev dev, char *show[], int output)
 			printf("\n");
 		first = 0;
 	}
+
+	if (num > 1)
+		return 0;
+	return BLKID_EXIT_NOTFOUND;
 }
 
 
@@ -903,8 +907,7 @@ int main(int argc, char **argv)
 
 		if ((dev = blkid_find_dev_with_tag(cache, search_type,
 						   search_value))) {
-			print_tags(dev, show, output_format);
-			err = 0;
+			err = print_tags(dev, show, output_format);
 		}
 	/* If we didn't specify a single device, show all available devices */
 	} else if (!numdev) {
@@ -916,11 +919,14 @@ int main(int argc, char **argv)
 		iter = blkid_dev_iterate_begin(cache);
 		blkid_dev_set_search(iter, search_type, search_value);
 		while (blkid_dev_next(iter, &dev) == 0) {
+			int tagerr;
+
 			dev = blkid_verify(cache, dev);
 			if (!dev)
 				continue;
-			print_tags(dev, show, output_format);
-			err = 0;
+			tagerr = print_tags(dev, show, output_format);
+			if (err != 0)
+				err = tagerr;
 		}
 		blkid_dev_iterate_end(iter);
 	/* Add all specified devices to cache (optionally display tags) */
@@ -929,12 +935,15 @@ int main(int argc, char **argv)
 						  BLKID_DEV_NORMAL);
 
 		if (dev) {
+			int tagerr;
+
 			if (search_type &&
 			    !blkid_dev_has_tag(dev, search_type,
 					       search_value))
 				continue;
-			print_tags(dev, show, output_format);
-			err = 0;
+			tagerr = print_tags(dev, show, output_format);
+			if (err != 0)
+				err = tagerr;
 		}
 	}
 
-- 
2.1.4

Reply via email to