[PATCH] isosize: iterate over all arguments even when something fails

[PATCH] isosize: iterate over all arguments even when something fails

[Sami Kerola]

Earlier the command exit too early if one of the arguments failed.  After
this change all arguments are examined, and command return value will have
information what happen during processing.

$ isosize /etc/shadow / ; echo $?
isosize: cannot open /etc/shadow: Permission denied
isosize: /: might not be an ISO filesystem
isosize: read error on /: Is a directory
14

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 disk-utils/isosize.8 | 24 ++++++++++++++++++++++++
 disk-utils/isosize.c | 38 ++++++++++++++++++++++++++------------
 2 files changed, 50 insertions(+), 12 deletions(-)

diff --git a/disk-utils/isosize.8 b/disk-utils/isosize.8
index 44ac124ea..225b0ae46 100644
--- a/disk-utils/isosize.8
+++ b/disk-utils/isosize.8
@@ -32,6 +32,30 @@ filesystem can be marginally larger than the actual size of the
 iso9660 filesystem.  One reason for this is that cd writers
 are allowed to add "run out" sectors at the end of an iso9660
 image.
+.SH "EXIT STATUS"
+Exit values are subject to bitwise inclusive or.  That means a return value
+such as 14 means all of the 2, 4, 8 errors happen to some of the input
+files.
+.PP
+.RS
+.PD 0
+.TP
+.B 0
+Success.
+.TP
+.B 1
+Generic error, such as invalid usage.
+.TP
+.B 2
+Could not open input file.
+.TP
+.B 4
+Invalid ISO header.
+.TP
+.B 8
+Size could not be read.
+.PD
+.RE
 .SH AVAILABILITY
 The isosize command is part of the util-linux package and is available from
 .UR https://\:www.kernel.org\:/pub\:/linux\:/utils\:/util-linux/
diff --git a/disk-utils/isosize.c b/disk-utils/isosize.c
index 5fbbbfc76..dfb2c4ca9 100644
--- a/disk-utils/isosize.c
+++ b/disk-utils/isosize.c
@@ -30,6 +30,12 @@
 #include "strutils.h"
 #include "closestream.h"
 
+enum {
+	EXIT_OPEN = 2,
+	EXIT_HEADER = 4,
+	EXIT_READ = 8
+};
+
 static int is_iso(int fd)
 {
 	char label[8];
@@ -86,22 +92,29 @@ static int isonum_733(unsigned char *p, int xflag)
 	return (le);
 }
 
-static void isosize(int argc, char *filenamep, int xflag, long divisor)
+static int isosize(int argc, char *filenamep, int xflag, long divisor)
 {
-	int fd, nsecs, ssize;
+	int fd, nsecs, ssize, ret = EXIT_SUCCESS;
 	unsigned char volume_space_size[8];
 	unsigned char logical_block_size[4];
 
-	if ((fd = open(filenamep, O_RDONLY)) < 0)
-		err(EXIT_FAILURE, _("cannot open %s"), filenamep);
-	if (is_iso(fd))
+	if ((fd = open(filenamep, O_RDONLY)) < 0) {
+		warn(_("cannot open %s"), filenamep);
+		return EXIT_OPEN;
+	}
+	if (is_iso(fd)) {
 		warnx(_("%s: might not be an ISO filesystem"), filenamep);
+		ret = EXIT_HEADER;
+	}
 
-	if (pread(fd, volume_space_size, sizeof(volume_space_size), 0x8050) <= 0 ||
-	    pread(fd, logical_block_size, sizeof(logical_block_size), 0x8080) <= 0) {
+	if (pread(fd, volume_space_size, sizeof(volume_space_size), 0x8050) != sizeof(volume_space_size) ||
+	    pread(fd, logical_block_size, sizeof(logical_block_size), 0x8080) != sizeof(logical_block_size)) {
 		if (errno)
-			err(EXIT_FAILURE, _("read error on %s"), filenamep);
-		errx(EXIT_FAILURE, _("read error on %s"), filenamep);
+			warn(_("read error on %s"), filenamep);
+		else
+			warnx(_("read error on %s"), filenamep);
+		close(fd);
+		return ret | EXIT_READ;
 	}
 
 	nsecs = isonum_733(volume_space_size, xflag);
@@ -122,8 +135,8 @@ static void isosize(int argc, char *filenamep, int xflag, long divisor)
 		else
 			printf("%lld\n", (product * ssize) / divisor);
 	}
-
 	close(fd);
+	return ret;
 }
 
 static void __attribute__((__noreturn__)) usage(void)
@@ -151,6 +164,7 @@ int main(int argc, char **argv)
 {
 	int j, ct, opt, xflag = 0;
 	long divisor = 0;
+	int ret = 0;
 
 	static const struct option longopts[] = {
 		{"divisor", required_argument, NULL, 'd'},
@@ -192,7 +206,7 @@ int main(int argc, char **argv)
 	}
 
 	for (j = optind; j < argc; j++)
-		isosize(ct, argv[j], xflag, divisor);
+		ret |= isosize(ct, argv[j], xflag, divisor);
 
-	return EXIT_SUCCESS;
+	return ret;
 }
-- 
2.14.1

[PATCH] look: use WORDLIST environment variable to find word list

[Sami Kerola]

The WORDLIST is the same hunspell(1) and ispell(1) uses to achieve the same.
Apparently aspell(1) does not work with files like traditional dict lists.

Reference: https://github.com/hunspell/hunspell/blob/master/man/hunspell.1#L388
Reference: http://www.skrenta.com/rt/man/ispell.1.html
Reference: http://aspell.net/man-html/Creating-an-Individual-Word-List.html#Creating-an-Individual-Word-List
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 misc-utils/look.1 | 5 +++++
 misc-utils/look.c | 7 ++++++-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/misc-utils/look.1 b/misc-utils/look.1
index 503e38584..cca83416e 100644
--- a/misc-utils/look.1
+++ b/misc-utils/look.1
@@ -101,6 +101,11 @@ sort -d /etc/passwd -o /tmp/look.dict
 look -t: root:foobar /tmp/look.dict
 .nf
 .RE
+.SH ENVIRONMENT
+.TP
+.B WORDLIST
+Path to a dictionary file.  The environment variable has greater priority
+than the dictionary path defined in FILES segment.
 .SH FILES
 .IP "\fB/usr/share/dict/words\fR" 4
 the dictionary
diff --git a/misc-utils/look.c b/misc-utils/look.c
index 60fbbbfca..00061f2a3 100644
--- a/misc-utils/look.c
+++ b/misc-utils/look.c
@@ -104,7 +104,12 @@ main(int argc, char *argv[])
 
 	setlocale(LC_ALL, "");
 
-	file = _PATH_WORDS;
+	if ((file = getenv("WORDLIST"))) {
+		if (access(file, R_OK))
+			err(EXIT_FAILURE, "WORDLIST=%s", file);
+	} else
+		file = _PATH_WORDS;
+
 	termchar = '\0';
 	string = NULL;		/* just for gcc */
 
-- 
2.14.1

Re: [PATCH] isosize: iterate over all arguments even when something fails

[Karel Zak]

On Sat, Aug 12, 2017 at 09:32:07AM +0100, Sami Kerola wrote:
> Earlier the command exit too early if one of the arguments failed.  After
> this change all arguments are examined, and command return value will have
> information what happen during processing.
> 
> $ isosize /etc/shadow / ; echo $?
> isosize: cannot open /etc/shadow: Permission denied
> isosize: /: might not be an ISO filesystem
> isosize: read error on /: Is a directory
> 14
> 
> Signed-off-by: Sami Kerola <kerolasa@iki.fi>
> ---
>  disk-utils/isosize.8 | 24 ++++++++++++++++++++++++
>  disk-utils/isosize.c | 38 ++++++++++++++++++++++++++------------
>  2 files changed, 50 insertions(+), 12 deletions(-)
> 
> diff --git a/disk-utils/isosize.8 b/disk-utils/isosize.8
> index 44ac124ea..225b0ae46 100644
> --- a/disk-utils/isosize.8
> +++ b/disk-utils/isosize.8
> @@ -32,6 +32,30 @@ filesystem can be marginally larger than the actual size of the
>  iso9660 filesystem.  One reason for this is that cd writers
>  are allowed to add "run out" sectors at the end of an iso9660
>  image.
> +.SH "EXIT STATUS"
> +Exit values are subject to bitwise inclusive or.  That means a return value
> +such as 14 means all of the 2, 4, 8 errors happen to some of the input
> +files.
> +.PP
> +.RS
> +.PD 0
> +.TP
> +.B 0
> +Success.
> +.TP
> +.B 1
> +Generic error, such as invalid usage.
> +.TP
> +.B 2
> +Could not open input file.
> +.TP
> +.B 4
> +Invalid ISO header.
> +.TP
> +.B 8
> +Size could not be read.
> +.PD
> +.RE
>  .SH AVAILABILITY
>  The isosize command is part of the util-linux package and is available from
>  .UR https://\:www.kernel.org\:/pub\:/linux\:/utils\:/util-linux/
> diff --git a/disk-utils/isosize.c b/disk-utils/isosize.c
> index 5fbbbfc76..dfb2c4ca9 100644
> --- a/disk-utils/isosize.c
> +++ b/disk-utils/isosize.c
> @@ -30,6 +30,12 @@
>  #include "strutils.h"
>  #include "closestream.h"
>  
> +enum {
> +	EXIT_OPEN = 2,
> +	EXIT_HEADER = 4,
> +	EXIT_READ = 8
> +};

Seems like overkill, why we need so specific information? I guess it
would be enough

    0 - EXIT_SUCCESS
    1 - EXIT_FAILURE
   64 - EXIT_SOMEOK  (already used for mount, lsblk, kill, ...)

 Karel

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

Re: [PATCH] look: use WORDLIST environment variable to find word list

[Karel Zak]

On Sat, Aug 12, 2017 at 09:32:08AM +0100, Sami Kerola wrote:
> The WORDLIST is the same hunspell(1) and ispell(1) uses to achieve the same.
> Apparently aspell(1) does not work with files like traditional dict lists.
> 
> Reference: https://github.com/hunspell/hunspell/blob/master/man/hunspell.1#L388
> Reference: http://www.skrenta.com/rt/man/ispell.1.html
> Reference: http://aspell.net/man-html/Creating-an-Individual-Word-List.html#Creating-an-Individual-Word-List
> Signed-off-by: Sami Kerola <kerolasa@iki.fi>
> ---
>  misc-utils/look.1 | 5 +++++
>  misc-utils/look.c | 7 ++++++-
>  2 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/misc-utils/look.1 b/misc-utils/look.1
> index 503e38584..cca83416e 100644
> --- a/misc-utils/look.1
> +++ b/misc-utils/look.1
> @@ -101,6 +101,11 @@ sort -d /etc/passwd -o /tmp/look.dict
>  look -t: root:foobar /tmp/look.dict
>  .nf
>  .RE
> +.SH ENVIRONMENT
> +.TP
> +.B WORDLIST
> +Path to a dictionary file.  The environment variable has greater priority
> +than the dictionary path defined in FILES segment.
>  .SH FILES
>  .IP "\fB/usr/share/dict/words\fR" 4
>  the dictionary
> diff --git a/misc-utils/look.c b/misc-utils/look.c
> index 60fbbbfca..00061f2a3 100644
> --- a/misc-utils/look.c
> +++ b/misc-utils/look.c
> @@ -104,7 +104,12 @@ main(int argc, char *argv[])
>  
>  	setlocale(LC_ALL, "");
>  
> -	file = _PATH_WORDS;
> +	if ((file = getenv("WORDLIST"))) {
> +		if (access(file, R_OK))
> +			err(EXIT_FAILURE, "WORDLIST=%s", file);
> +	} else
> +		file = _PATH_WORDS;
> +


Would be better fallback to _PATH_WORDS if WORDLIST= points to
nowhere?

    file = _PATH_WORDS; /* default */
    if ((x = getenv("WORDLIST")) && access(file, R_OK) == 0)
        file = x;

seems like more backwardly compatible way.

    Karel

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

Re: [PATCH] look: use WORDLIST environment variable to find word list

[Sami Kerola]

On Mon, 14 Aug 2017, Karel Zak wrote:

> Would be better fallback to _PATH_WORDS if WORDLIST= points to
> nowhere?
> 
>     file = _PATH_WORDS; /* default */
>     if ((x = getenv("WORDLIST")) && access(file, R_OK) == 0)
>         file = x;
> 
> seems like more backwardly compatible way.

No problems, change is updated much like proposed above.

https://github.com/kerolasa/lelux-utiliteetit/commit/dd8be07d84988b130286a1194210bdb26b223736

That is in branch: 2017wk32

-- 
Sami Kerola
http://www.iki.fi/kerolasa/

Re: [PATCH] isosize: iterate over all arguments even when something fails

[Sami Kerola]

On Mon, 14 Aug 2017, Karel Zak wrote:
> Seems like overkill, why we need so specific information? I guess it
> would be enough
> 
>     0 - EXIT_SUCCESS
>     1 - EXIT_FAILURE
>    64 - EXIT_SOMEOK  (already used for mount, lsblk, kill, ...)

I kept small overkill and used two different return values: 64 for some 
ok, and 32 all failed.

https://github.com/kerolasa/lelux-utiliteetit/commit/dea7a778beb6b043a889904d43b2b2b561814326

That is in branch: 2017wk32

-- 
Sami Kerola
http://www.iki.fi/kerolasa/

Re: [PATCH] look: use WORDLIST environment variable to find word list

[Karel Zak]

On Tue, Aug 15, 2017 at 07:13:34PM +0100, Sami Kerola wrote:
> On Mon, 14 Aug 2017, Karel Zak wrote:
> 
> > Would be better fallback to _PATH_WORDS if WORDLIST= points to
> > nowhere?
> > 
> >     file = _PATH_WORDS; /* default */
> >     if ((x = getenv("WORDLIST")) && access(file, R_OK) == 0)
> >         file = x;
> > 
> > seems like more backwardly compatible way.
> 
> No problems, change is updated much like proposed above.
> 
> https://github.com/kerolasa/lelux-utiliteetit/commit/dd8be07d84988b130286a1194210bdb26b223736
> 
> That is in branch: 2017wk32

Applied, thanks.

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

Re: [PATCH] isosize: iterate over all arguments even when something fails

[Karel Zak]

On Tue, Aug 15, 2017 at 07:23:39PM +0100, Sami Kerola wrote:
> On Mon, 14 Aug 2017, Karel Zak wrote:
> > Seems like overkill, why we need so specific information? I guess it
> > would be enough
> > 
> >     0 - EXIT_SUCCESS
> >     1 - EXIT_FAILURE
> >    64 - EXIT_SOMEOK  (already used for mount, lsblk, kill, ...)
> 
> I kept small overkill and used two different return values: 64 for some 
> ok, and 32 all failed.

OK, I have merged a little different way how to implement it.

    Karel


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