[PATCH] Print error message if failing to write super for 1.x metadata

[PATCH] Print error message if failing to write super for 1.x metadata

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

In addition remove attempt to print an error message if
write_init_super() fails, as this is handled in the various
write_init_super() functions. This avoids a segfault on error.

Reported by Jim Meyering in
https://bugzilla.redhat.com/show_bug.cgi?id=795461

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 Create.c |    3 ---
 super1.c |   20 ++++++++++++++++----
 2 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/Create.c b/Create.c
index 90ff3ed..e5c6b05 100644
--- a/Create.c
+++ b/Create.c
@@ -924,9 +924,6 @@ int Create(struct supertype *st, char *mddev,
 			}
 
 			if (st->ss->write_init_super(st)) {
-				fprintf(stderr,
-					Name ": Failed to write metadata to %s\n",
-					dv->devname);
 				st->ss->free_super(st);
 				goto abort_locked;
 			}
diff --git a/super1.c b/super1.c
index a18952a..1db4de6 100644
--- a/super1.c
+++ b/super1.c
@@ -1106,13 +1106,16 @@ static int write_init_super1(struct supertype *st)
 		}
 		free(refst);
 
-		if (!get_dev_size(di->fd, NULL, &dsize))
-			return 1;
+		if (!get_dev_size(di->fd, NULL, &dsize)) {
+			rv = 1;
+			goto error_out;
+		}
 		dsize >>= 9;
 
 		if (dsize < 24) {
 			close(di->fd);
-			return 2;
+			rv = 2;
+			goto error_out;
 		}
 
 
@@ -1176,7 +1179,11 @@ static int write_init_super1(struct supertype *st)
 			sb->data_size = __cpu_to_le64(dsize - reserved);
 			break;
 		default:
-			return -EINVAL;
+			fprintf(stderr,	Name ": Failed to write invalid "
+				"metadata format 1.%i to %s\n",
+				st->minor_version, di->devname);
+			rv = -EINVAL;
+			goto out;
 		}
 
 
@@ -1192,6 +1199,11 @@ static int write_init_super1(struct supertype *st)
 		close(di->fd);
 		di->fd = -1;
 	}
+error_out:
+	if (rv)
+		fprintf(stderr,	Name ": Failed to write metadata to %s\n",
+			di->devname);
+out:
 	return rv;
 }
 #endif
-- 
1.7.7.6

Re: [PATCH] Print error message if failing to write super for 1.x metadata

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 2385 bytes --]

On Wed, 22 Feb 2012 12:01:56 +0100 Jes.Sorensen@redhat.com wrote:

> From: Jes Sorensen <Jes.Sorensen@redhat.com>
> 
> In addition remove attempt to print an error message if
> write_init_super() fails, as this is handled in the various
> write_init_super() functions. This avoids a segfault on error.
> 
> Reported by Jim Meyering in
> https://bugzilla.redhat.com/show_bug.cgi?id=795461
> 
> Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>

Applied, thanks.

Though there was already a case which printed an error which you left
unchanged so now in some cases it would print the same message twice.
So I removed the old printf.

Thanks,
NeilBrown


> ---
>  Create.c |    3 ---
>  super1.c |   20 ++++++++++++++++----
>  2 files changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/Create.c b/Create.c
> index 90ff3ed..e5c6b05 100644
> --- a/Create.c
> +++ b/Create.c
> @@ -924,9 +924,6 @@ int Create(struct supertype *st, char *mddev,
>  			}
>  
>  			if (st->ss->write_init_super(st)) {
> -				fprintf(stderr,
> -					Name ": Failed to write metadata to %s\n",
> -					dv->devname);
>  				st->ss->free_super(st);
>  				goto abort_locked;
>  			}
> diff --git a/super1.c b/super1.c
> index a18952a..1db4de6 100644
> --- a/super1.c
> +++ b/super1.c
> @@ -1106,13 +1106,16 @@ static int write_init_super1(struct supertype *st)
>  		}
>  		free(refst);
>  
> -		if (!get_dev_size(di->fd, NULL, &dsize))
> -			return 1;
> +		if (!get_dev_size(di->fd, NULL, &dsize)) {
> +			rv = 1;
> +			goto error_out;
> +		}
>  		dsize >>= 9;
>  
>  		if (dsize < 24) {
>  			close(di->fd);
> -			return 2;
> +			rv = 2;
> +			goto error_out;
>  		}
>  
>  
> @@ -1176,7 +1179,11 @@ static int write_init_super1(struct supertype *st)
>  			sb->data_size = __cpu_to_le64(dsize - reserved);
>  			break;
>  		default:
> -			return -EINVAL;
> +			fprintf(stderr,	Name ": Failed to write invalid "
> +				"metadata format 1.%i to %s\n",
> +				st->minor_version, di->devname);
> +			rv = -EINVAL;
> +			goto out;
>  		}
>  
>  
> @@ -1192,6 +1199,11 @@ static int write_init_super1(struct supertype *st)
>  		close(di->fd);
>  		di->fd = -1;
>  	}
> +error_out:
> +	if (rv)
> +		fprintf(stderr,	Name ": Failed to write metadata to %s\n",
> +			di->devname);
> +out:
>  	return rv;
>  }
>  #endif


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH] Print error message if failing to write super for 1.x metadata

[Jes Sorensen]

On 02/22/12 22:58, NeilBrown wrote:
> On Wed, 22 Feb 2012 12:01:56 +0100 Jes.Sorensen@redhat.com wrote:
> 
>> > From: Jes Sorensen <Jes.Sorensen@redhat.com>
>> > 
>> > In addition remove attempt to print an error message if
>> > write_init_super() fails, as this is handled in the various
>> > write_init_super() functions. This avoids a segfault on error.
>> > 
>> > Reported by Jim Meyering in
>> > https://bugzilla.redhat.com/show_bug.cgi?id=795461
>> > 
>> > Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
> Applied, thanks.
> 
> Though there was already a case which printed an error which you left
> unchanged so now in some cases it would print the same message twice.
> So I removed the old printf.

Whoops, thanks for catching that.

Cheers,
Jes