* [Cluster-devel] tunegfs2: Fix usage and ensure we don't try to open a null device
@ 2011-07-14 10:23 Steven Whitehouse
2011-07-14 10:52 ` Andrew Price
0 siblings, 1 reply; 3+ messages in thread
From: Steven Whitehouse @ 2011-07-14 10:23 UTC (permalink / raw)
To: cluster-devel.redhat.com
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);
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [Cluster-devel] tunegfs2: Fix usage and ensure we don't try to open a null device
2011-07-14 10:23 [Cluster-devel] tunegfs2: Fix usage and ensure we don't try to open a null device Steven Whitehouse
@ 2011-07-14 10:52 ` Andrew Price
2011-07-14 11:18 ` Steven Whitehouse
0 siblings, 1 reply; 3+ messages in thread
From: Andrew Price @ 2011-07-14 10:52 UTC (permalink / raw)
To: cluster-devel.redhat.com
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<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);
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* [Cluster-devel] tunegfs2: Fix usage and ensure we don't try to open a null device
2011-07-14 10:52 ` Andrew Price
@ 2011-07-14 11:18 ` Steven Whitehouse
0 siblings, 0 replies; 3+ messages in thread
From: Steven Whitehouse @ 2011-07-14 11:18 UTC (permalink / raw)
To: cluster-devel.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);
> >
> >
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2011-07-14 11:18 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-07-14 10:23 [Cluster-devel] tunegfs2: Fix usage and ensure we don't try to open a null device Steven Whitehouse
2011-07-14 10:52 ` Andrew Price
2011-07-14 11:18 ` Steven Whitehouse
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).