From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Price Date: Thu, 14 Jul 2011 11:52:12 +0100 Subject: [Cluster-devel] tunegfs2: Fix usage and ensure we don't try to open a null device In-Reply-To: <1310638990.2761.6.camel@menhir> References: <1310638990.2761.6.camel@menhir> Message-ID: <4E1ECA5C.7000400@redhat.com> List-Id: To: cluster-devel.redhat.com MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Hi Steve, This looks good to me. For consistency, should we start using sysexits.h values throughout gfs2-utils? Andy 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 > Reported-by: Nathan Straz > > 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 > #include > #include > +#include > > #include > #include > @@ -51,8 +52,8 @@ void parse_mount_options(char *arg) > > static void usage(char *name) > { > - printf("Usage: %s -L -U -l -o " > - " \n", basename(name)); > + printf("Usage: %s [-hlV] [-L] [-U]\n\t" > + "[-o] \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 > #include > - > +#include > #include > #include > #include > @@ -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); > >