From mboxrd@z Thu Jan 1 00:00:00 1970 From: Steven Whitehouse Date: Thu, 14 Jul 2011 12:18:06 +0100 Subject: [Cluster-devel] tunegfs2: Fix usage and ensure we don't try to open a null device In-Reply-To: <4E1ECA5C.7000400@redhat.com> References: <1310638990.2761.6.camel@menhir> <4E1ECA5C.7000400@redhat.com> Message-ID: <1310642286.2761.8.camel@menhir> List-Id: To: cluster-devel.redhat.com MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit 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 > > 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); > > > > >