[Cluster-devel] tunegfs2: Fix usage and ensure we don't try to open a null device

[Steven Whitehouse <swhiteho@redhat.com>]

Hi,

On Thu, 2011-07-14 at 11:52 +0100, Andrew Price wrote:
> Hi Steve,
> 
> This looks good to me. For consistency, should we start using sysexits.h 
> values throughout gfs2-utils?
> 
> Andy
> 
Yes, thats not a bad plan, although I think fsck has some specific
values it has to use which may or may not match sysexits,

Steve.

> On 14/07/11 11:23, Steven Whitehouse wrote:
> >
> > This is designed to address Red Hat bz #719124 and #719126
> >
> > The help message is updated to include all supported options.
> > Also, the return codes are now taken from sysexits.h rather
> > than trying to pass negative errno values as per kernel
> > code. There is not an exact error code for all potential
> > events, but they are close enough I think.
> >
> > The code also checks that there is exactly one non-option
> > argument (i.e. the device) given before attempting to open
> > it.
> >
> > Signed-off-by: Steven Whitehouse<swhiteho@redhat.com>
> > Reported-by: Nathan Straz<nstraz@redhat.com>
> >
> > diff --git a/gfs2/tune/main.c b/gfs2/tune/main.c
> > index 48fd59f..6a0daff 100644
> > --- a/gfs2/tune/main.c
> > +++ b/gfs2/tune/main.c
> > @@ -1,6 +1,7 @@
> >   #include<stdio.h>
> >   #include<stdlib.h>
> >   #include<libgen.h>
> > +#include<sysexits.h>
> >
> >   #include<sys/types.h>
> >   #include<sys/stat.h>
> > @@ -51,8 +52,8 @@ void parse_mount_options(char *arg)
> >
> >   static void usage(char *name)
> >   {
> > -	printf("Usage: %s -L<volume label>  -U<UUID>  -l -o "
> > -		"<mount options>  <device>  \n", basename(name));
> > +	printf("Usage: %s [-hlV] [-L<volume_label>] [-U<UUID>]\n\t"
> > +		"[-o<mount_options>]<device>  \n", basename(name));
> >   }
> >
> >   static void version(void)
> > @@ -62,14 +63,14 @@ static void version(void)
> >
> >   int main(int argc, char **argv)
> >   {
> > -	int c, status = 0;
> > +	int c, status;
> >
> >   	memset(tfs, 0, sizeof(struct tunegfs2));
> >   	while((c = getopt(argc, argv, "hL:U:lo:V")) != -1) {
> >   		switch(c) {
> >   		case 'h':
> >   			usage(argv[0]);
> > -			break;
> > +			return 0;
> >   		case 'L':
> >   			tfs->opt_label = 1;
> >   			tfs->label = optarg;
> > @@ -86,23 +87,27 @@ int main(int argc, char **argv)
> >   			break;
> >   		case 'V':
> >   			version();
> > -			break;
> > +			return 0;
> >   		default:
> >   			fprintf(stderr, _("Invalid option.\n"));
> >   			usage(argv[0]);
> > -			status = -EINVAL;
> > -			goto out;
> > +			return EX_USAGE;
> >   		}
> >   	}
> >
> > +	if ((argc - optind) != 1) {
> > +		fprintf(stderr, _("Incorrect number of arguments\n"));
> > +		usage(argv[0]);
> > +		return EX_USAGE;
> > +	}
> > +
> >   	tfs->devicename = argv[optind];
> >   	tfs->fd = open(tfs->devicename, O_RDWR);
> >
> >   	if (tfs->fd<  0) {
> >   		fprintf(stderr, _("Unable to open device %s\n"),
> >   				tfs->devicename);
> > -		status = -EIO;
> > -		goto out;
> > +		return EX_IOERR;
> >   	}
> >
> >   	status = read_super(tfs);
> > diff --git a/gfs2/tune/super.c b/gfs2/tune/super.c
> > index 056e868..9d67054 100644
> > --- a/gfs2/tune/super.c
> > +++ b/gfs2/tune/super.c
> > @@ -1,6 +1,6 @@
> >   #include<stdio.h>
> >   #include<stdlib.h>
> > -
> > +#include<sysexits.h>
> >   #include<sys/types.h>
> >   #include<sys/stat.h>
> >   #include<errno.h>
> > @@ -63,7 +63,7 @@ static int str2uuid(const char *newval, char *uuid)
> >
> >   	if (strlen(newval) != 36) {
> >   		fprintf(stderr, _("Invalid UUID specified.\n"));
> > -		return -EINVAL;
> > +		return EX_DATAERR;
> >   	}
> >
> >   	cp = uuid;
> > @@ -74,13 +74,13 @@ static int str2uuid(const char *newval, char *uuid)
> >   				continue;
> >   			fprintf(stderr, _("uuid %s has an invalid format."),
> >   					newval);
> > -			return -EINVAL;
> > +			return EX_DATAERR;
> >   		}
> >   		if (!isxdigit(newval[i])) {
> >   			fprintf(stderr, _("uuid %s has an invalid hex "
> >   						"digit '%c' at offset %d.\n"),
> >   					newval, newval[i], i + 1);
> > -			return -EINVAL;
> > +			return EX_DATAERR;
> >   		}
> >   		*cp = str_to_hexchar(&newval[i++]);
> >   		cp++;
> > @@ -96,13 +96,13 @@ int read_super(struct tunegfs2 *tfs)
> >   	block = malloc(sizeof(char) * GFS2_DEFAULT_BSIZE);
> >   	n = pread(tfs->fd, block, GFS2_DEFAULT_BSIZE, tfs->sb_start);
> >   	if (n<  0) {
> > -		fprintf(stderr, _("Error reading from device"));
> > -		return errno;
> > +		perror("read_super: pread");
> > +		return EX_IOERR;
> >   	}
> >   	tfs->sb = block;
> >   	if (be32_to_cpu(tfs->sb->sb_header.mh_magic) != GFS2_MAGIC) {
> >   		fprintf(stderr, _("Not a GFS/GFS2 device\n"));
> > -		return -EINVAL;
> > +		return EX_IOERR;
> >   	}
> >   	return 0;
> >   }
> > @@ -144,9 +144,9 @@ int write_super(const struct tunegfs2 *tfs)
> >   {
> >   	int n;
> >   	n = pwrite(tfs->fd, tfs->sb, GFS2_DEFAULT_BSIZE, tfs->sb_start);
> > -	if (n<0) {
> > -		fprintf(stderr, _("Unable to write super block\n"));
> > -		return -errno;
> > +	if (n<  0) {
> > +		perror("write_super: pwrite");
> > +		return EX_IOERR;
> >   	}
> >   	return 0;
> >   }
> > @@ -164,7 +164,7 @@ int change_label(struct tunegfs2 *tfs, const char *fsname)
> >   	}
> >   	if (fsname_len<  l) {
> >   		fprintf(stderr, _("Label too long\n"));
> > -		return -E2BIG;
> > +		return EX_DATAERR;
> >   	}
> >   	memset(sb_fsname, '\0', fsname_len);
> >   	memcpy(sb_fsname, fsname, l);
> > @@ -178,7 +178,7 @@ int change_uuid(struct tunegfs2 *tfs, const char *str)
> >   	if (be32_to_cpu(tfs->sb->sb_header.mh_magic) != GFS2_MAGIC) {
> >   		fprintf(stderr, _("UUID can be changed for a GFS2"));
> >   		fprintf(stderr, _(" device only\n"));
> > -		return -EINVAL;
> > +		return EX_IOERR;
> >   	}
> >   	status = str2uuid(str, uuid);
> >   	if (!status)
> > @@ -193,7 +193,7 @@ int change_lockproto(struct tunegfs2 *tfs, const char *lockproto)
> >   	if (strncmp(lockproto, "lock_dlm", 8)
> >   			&&  strncmp(lockproto, "lock_nolock", 11)) {
> >   		fprintf(stderr, _("Incorrect lock protocol specified\n"));
> > -		return -EINVAL;
> > +		return EX_DATAERR;
> >   	}
> >   	memset(tfs->sb->sb_lockproto, '\0', GFS2_LOCKNAME_LEN);
> >   	strncpy(tfs->sb->sb_lockproto, lockproto, l);
> > @@ -220,7 +220,7 @@ int change_locktable(struct tunegfs2 *tfs, const char *locktable)
> >
> >   	if (l>  GFS2_LOCKNAME_LEN - fsname_len - 1) {
> >   		fprintf(stderr, _("Lock table name too big\n"));
> > -		return -E2BIG;
> > +		return EX_DATAERR;
> >   	}
> >   	memset(t_fsname, '\0', GFS2_LOCKNAME_LEN);
> >   	strncpy(t_fsname, sb_fsname, fsname_len);
> >
> >
>