[Cluster-devel] [PATCH RHEL6 1/2] tunegfs2: Ensure we don't try to open a null device

[Cluster-devel] [PATCH RHEL6 1/2] tunegfs2: Ensure we don't try to open a null device

[Andrew Price]

This is based on upstream commit a830c8747cf7dcc899dc92ad13c3a3b1a3738092

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.

rhbz#719124

Signed-off-by: Andrew Price <anprice@redhat.com>
Signed-off-by: Steven Whitehouse <swhiteho@redhat.com>
Reported-by: Nathan Straz <nstraz@redhat.com>
---
 gfs2/tune/main.c  |   19 ++++++++++++-------
 gfs2/tune/super.c |   28 ++++++++++++++--------------
 2 files changed, 26 insertions(+), 21 deletions(-)

diff --git a/gfs2/tune/main.c b/gfs2/tune/main.c
index 48fd59f..be542d7 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>
@@ -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 e7ae4be..f02edd6 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);
-- 
1.7.1

[Cluster-devel] [PATCH RHEL6 2/2] tunegfs2: Fix usage message

[Andrew Price]

This is based on upstream commit a830c8747cf7dcc899dc92ad13c3a3b1a3738092

The help message is updated to include all supported options.

rhbz#719126

Signed-off-by: Steven Whitehouse <swhiteho@redhat.com>
Signed-off-by: Andrew Price <anprice@redhat.com>
Reported-by: Nathan Straz <nstraz@redhat.com>
---
 gfs2/tune/main.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/gfs2/tune/main.c b/gfs2/tune/main.c
index be542d7..6a0daff 100644
--- a/gfs2/tune/main.c
+++ b/gfs2/tune/main.c
@@ -52,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)
-- 
1.7.1

[Cluster-devel] [PATCH RHEL6 2/2] tunegfs2: Fix usage message

[Bob Peterson]

----- Original Message -----
| This is based on upstream commit
| a830c8747cf7dcc899dc92ad13c3a3b1a3738092
| 
| The help message is updated to include all supported options.
| 
| rhbz#719126
| 
| Signed-off-by: Steven Whitehouse <swhiteho@redhat.com>
| Signed-off-by: Andrew Price <anprice@redhat.com>
| Reported-by: Nathan Straz <nstraz@redhat.com>
| ---

Hi,

ACK,

Regards,

Bob Peterson
Red Hat File Systems

[Cluster-devel] [PATCH RHEL6 2/2] tunegfs2: Fix usage message

[Steven Whitehouse]

Hi,

Both look good to me,

Steve.

On Mon, 2011-07-25 at 18:03 +0100, Andrew Price wrote:
> This is based on upstream commit a830c8747cf7dcc899dc92ad13c3a3b1a3738092
> 
> The help message is updated to include all supported options.
> 
> rhbz#719126
> 
> Signed-off-by: Steven Whitehouse <swhiteho@redhat.com>
> Signed-off-by: Andrew Price <anprice@redhat.com>
> Reported-by: Nathan Straz <nstraz@redhat.com>
> ---
>  gfs2/tune/main.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/gfs2/tune/main.c b/gfs2/tune/main.c
> index be542d7..6a0daff 100644
> --- a/gfs2/tune/main.c
> +++ b/gfs2/tune/main.c
> @@ -52,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)

[Cluster-devel] [PATCH RHEL6 1/2] tunegfs2: Ensure we don't try to open a null device

[Bob Peterson]

----- Original Message -----
| This is based on upstream commit
| a830c8747cf7dcc899dc92ad13c3a3b1a3738092
| 
| 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.
| 
| rhbz#719124
| 
| Signed-off-by: Andrew Price <anprice@redhat.com>
| Signed-off-by: Steven Whitehouse <swhiteho@redhat.com>
| Reported-by: Nathan Straz <nstraz@redhat.com>

Hi,

Looks good.  ACK.

Bob Peterson
Red Hat File Systems