Proposed extension to stat(1)

[Gleb Kurtsou]

On (07/05/2014 11:12), Thomas Quinot wrote:
> I'm proposing the addition of a command line switch -H to stat(1),
> causing arguments to be interpreted as NFS file handles instead of file
> names. This comes in handy when investigating NFS load issues.
> 
> (A further possible extension would be to optionally include information
> from statfs/fhstatfs, but that's next on my list :) ).
> 
> Thomas.
> 
> Index: usr.bin/stat/stat.1
> ===================================================================
> --- usr.bin/stat/stat.1	(révision 265192)
> +++ usr.bin/stat/stat.1	(copie de travail)
> @@ -38,7 +38,7 @@
>  .Nd display file status
>  .Sh SYNOPSIS
>  .Nm
> -.Op Fl FLnq
> +.Op Fl FHLnq
>  .Op Fl f Ar format | Fl l | r | s | x
>  .Op Fl t Ar timefmt
>  .Op Ar
> @@ -124,6 +124,12 @@
>  .Fl F
>  implies
>  .Fl l .
> +.It Fl H
> +Treat each argument as the hexadecimal representation of an NFS file handle,
> +and use
> +.Xr fhstat 2
> +instead of
> +.Xr lstat 2 .

Noting that it requires root may be helpful. 'each argument' confused me
a lot.

>  .It Fl L
>  Use
>  .Xr stat 2
> Index: usr.bin/stat/stat.c
> ===================================================================
> --- usr.bin/stat/stat.c	(révision 265192)
> +++ usr.bin/stat/stat.c	(copie de travail)
> @@ -50,6 +50,7 @@
>  #include <sys/param.h>
>  #include <sys/types.h>
>  #include <sys/stat.h>
> +#include <sys/mount.h>
>  
>  #include <ctype.h>
>  #include <err.h>
> @@ -203,9 +204,10 @@
>  {
>  	struct stat st;
>  	int ch, rc, errs, am_readlink;
> -	int lsF, fmtchar, usestat, fn, nonl, quiet;
> +	int lsF, fmtchar, usestat, nfs_handle, fn, nonl, quiet;
>  	const char *statfmt, *options, *synopsis;
>  	char dname[sizeof _PATH_DEV + SPECNAMELEN] = _PATH_DEV;
> +        fhandle_t fhnd;

whitespace?

>  	const char *file;
>  
>  	am_readlink = 0;
> @@ -212,6 +214,7 @@
>  	lsF = 0;
>  	fmtchar = '\0';
>  	usestat = 0;
> +        nfs_handle = 0;

whitespace?

>  	nonl = 0;
>  	quiet = 0;
>  	linkfail = 0;
> @@ -226,9 +229,9 @@
>  		fmtchar = 'f';
>  		quiet = 1;
>  	} else {
> -		options = "f:FlLnqrst:x";
> +		options = "f:FHlLnqrst:x";
>  		synopsis = "[-FLnq] [-f format | -l | -r | -s | -x] "
> -		    "[-t timefmt] [file ...]";
> +		    "[-t timefmt] [file|handle ...]";
>  	}
>  
>  	while ((ch = getopt(argc, argv, options)) != -1)
> @@ -236,6 +239,9 @@
>  		case 'F':
>  			lsF = 1;
>  			break;
> +                case 'H':
> +			nfs_handle = 1;
> +			break;

Whitespace.

>  		case 'L':
>  			usestat = 1;
>  			break;
> @@ -320,8 +326,33 @@
>  				file = "(stdin)";
>  			rc = fstat(STDIN_FILENO, &st);
>  		} else {
> +			int j;
> +			char *inval;
> +
>  			file = argv[0];
> -			if (usestat) {
> +			if (nfs_handle) {
> +				rc = 0;
> +				bzero (&fhnd, sizeof fhnd);

Style. Extra space after bzero.

> +				j = MIN(2 * sizeof fhnd, strlen(file));

sizeof style.

> +				if (j & 1) {
> +					rc = -1;
> +				} else {
> +					while (j) {
> +						((char*) &fhnd)[j / 2 - 1] = strtol(&file[j - 2], &inval, 16);

It's badly broken:
- Result for excessive "01" vs "0102" will differ in
  strlen(file) > sizeof(fhnd) * 2 case.
- Will "+1-2" be considered a valid file handle?
- why do we care about strlen(file) != sizeof(fh) case?


> +						if (inval != NULL) {

According to strtol(3) such error handling is insufficient.
At least it won't work for strlen(file) > sizeof(fhnd) * 2 case.


> +							rc = -1;
> +							break;
> +						}
> +						argv[0][j - 2] = '\0';

First we initialize file = argv[0].
Then we set every third character in argv[0] to '\0'.
Afterwards file is used to print error message or actual result.

> +						j -= 2;
> +					}
> +					if (!rc)

style. rc is not bool.

> +						rc = fhstat(&fhnd, &st);
> +					else
> +						errno = EINVAL;
> +				}
> +
> +			} else if (usestat) {
>  				/*
>  				 * Try stat() and if it fails, fall back to
>  				 * lstat() just in case we're examining a