[PATCH 0/2] xfsdump: 2 more fixes

[PATCH 0/2] xfsdump: 2 more fixes

[Eric Sandeen]

Related to the previous one.

patch one avoids the segfault if for some reason (i.e. bug) the partials
array fills.

patch two fixes the #ifdef DEBUGPARTIALS build.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 1/2] xfsdump: avoid segfault in partial_reg() in error case

[Eric Sandeen]

If we go down the "/* Should never get here. */" path
in partial_reg(), we issue a warning but then continue
with the function.  This calls pi_unlock() twice,
but worse, uses a null isptr:

        if ( ! isptr ) {
... isptr is never set if we get to ...
                /* Should never get here. */
                pi_unlock();
...
        }
...
        /* Update this drive's entry */
        bsptr = &isptr->is_bs[d_index];
        if (bsptr->endoffset == 0) {

>From all appearances, because we unlock on that "never get
here" path, it should just be returning after printing the
warning.  So add that, and we avoid the segfault.

The previous fix to partial_reg() should prevent us from
hitting this in the first place.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

diff --git a/restore/content.c b/restore/content.c
index 54d933c..cc49336 100644
--- a/restore/content.c
+++ b/restore/content.c
@@ -9007,6 +9007,7 @@ partial_reg( ix_t d_index,
 #ifdef DEBUGPARTIALS
 		dump_partials();
 #endif
+		return;
 	}
 
 found:

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 2/2] xfsdump: fix DEBUGPARTIALS build

[Eric Sandeen]

the DEBUGPARTIALS debug code might have been helpful
in this saga, so get it building again.

The primary build failure is that STREAM_MAX isn't
defined for the num_partials[STREAM_MAX] array;
the loop which uses that array iterates "drivecnt"
times, so just allocate an array of that size.

Fix a few printf format warnings while we're at it.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

diff --git a/restore/content.c b/restore/content.c
index cc49336..8ad0f00 100644
--- a/restore/content.c
+++ b/restore/content.c
@@ -8857,22 +8857,23 @@ dump_partials(void)
 	int i;
 
 	pi_lock();
-	printf("\npartial_reg: count=%d\n", persp->a.parrestcnt);
+	printf("\npartial_reg: count=%d\n", (int)persp->a.parrestcnt);
 	if (persp->a.parrestcnt > 0) {
 		for (i=0; i < partialmax; i++ ) {
 			if (persp->a.parrest[i].is_ino > 0) {
 				int j;
 
 				isptr = &persp->a.parrest[i];
-				printf( "\tino=%lld ", isptr->is_ino);
+				printf("\tino=%llu ",
+				       (unsigned long long)isptr->is_ino);
 				for (j=0, bsptr=isptr->is_bs;
 				     j < drivecnt; 
 				     j++, bsptr++)
 				{
 					if (bsptr->endoffset > 0) {
 						printf("%d:%lld-%lld ",
-						     j, bsptr->offset, 
-						     bsptr->endoffset);
+						   j, (long long)bsptr->offset,
+						   (long long)bsptr->endoffset);
 					} 
 				}
 				printf( "\n");
@@ -8892,13 +8893,17 @@ dump_partials(void)
 void
 check_valid_partials(void)
 {
-        int num_partials[STREAM_MAX]; /* sum of partials for a given drive */
+	int *num_partials; /* array for sum of partials for a given drive */
 	partial_rest_t *isptr = NULL;
 	bytespan_t *bsptr = NULL;
 	int i;
 
 	/* zero the sums for each stream */
-        memset(num_partials, 0, sizeof(num_partials));
+	num_partials = calloc(drivecnt, sizeof(int));
+	if (!num_partials) {
+		perror("num_partials array allocation");
+		return;
+	}
 
 	pi_lock();
 	if (persp->a.parrestcnt > 0) {
@@ -8926,6 +8931,7 @@ check_valid_partials(void)
 		}
 	}
 	pi_unlock();
+	free(num_partials);
 }
 #endif
 


_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 1/2] xfsdump: avoid segfault in partial_reg() in error case

[Carlos Maiolino]

Looks good to me

Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>

On Tue, Oct 08, 2013 at 05:05:54PM -0500, Eric Sandeen wrote:
> If we go down the "/* Should never get here. */" path
> in partial_reg(), we issue a warning but then continue
> with the function.  This calls pi_unlock() twice,
> but worse, uses a null isptr:
> 
>         if ( ! isptr ) {
> ... isptr is never set if we get to ...
>                 /* Should never get here. */
>                 pi_unlock();
> ...
>         }
> ...
>         /* Update this drive's entry */
>         bsptr = &isptr->is_bs[d_index];
>         if (bsptr->endoffset == 0) {
> 
> From all appearances, because we unlock on that "never get
> here" path, it should just be returning after printing the
> warning.  So add that, and we avoid the segfault.
> 
> The previous fix to partial_reg() should prevent us from
> hitting this in the first place.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> 
> diff --git a/restore/content.c b/restore/content.c
> index 54d933c..cc49336 100644
> --- a/restore/content.c
> +++ b/restore/content.c
> @@ -9007,6 +9007,7 @@ partial_reg( ix_t d_index,
>  #ifdef DEBUGPARTIALS
>  		dump_partials();
>  #endif
> +		return;
>  	}
>  
>  found:
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

-- 
Carlos

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 2/2] xfsdump: fix DEBUGPARTIALS build

[Carlos Maiolino]

Looks good to me indeed

Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
On Tue, Oct 08, 2013 at 05:31:41PM -0500, Eric Sandeen wrote:
> the DEBUGPARTIALS debug code might have been helpful
> in this saga, so get it building again.
> 
> The primary build failure is that STREAM_MAX isn't
> defined for the num_partials[STREAM_MAX] array;
> the loop which uses that array iterates "drivecnt"
> times, so just allocate an array of that size.
> 
> Fix a few printf format warnings while we're at it.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> 
> diff --git a/restore/content.c b/restore/content.c
> index cc49336..8ad0f00 100644
> --- a/restore/content.c
> +++ b/restore/content.c
> @@ -8857,22 +8857,23 @@ dump_partials(void)
>  	int i;
>  
>  	pi_lock();
> -	printf("\npartial_reg: count=%d\n", persp->a.parrestcnt);
> +	printf("\npartial_reg: count=%d\n", (int)persp->a.parrestcnt);
>  	if (persp->a.parrestcnt > 0) {
>  		for (i=0; i < partialmax; i++ ) {
>  			if (persp->a.parrest[i].is_ino > 0) {
>  				int j;
>  
>  				isptr = &persp->a.parrest[i];
> -				printf( "\tino=%lld ", isptr->is_ino);
> +				printf("\tino=%llu ",
> +				       (unsigned long long)isptr->is_ino);
>  				for (j=0, bsptr=isptr->is_bs;
>  				     j < drivecnt; 
>  				     j++, bsptr++)
>  				{
>  					if (bsptr->endoffset > 0) {
>  						printf("%d:%lld-%lld ",
> -						     j, bsptr->offset, 
> -						     bsptr->endoffset);
> +						   j, (long long)bsptr->offset,
> +						   (long long)bsptr->endoffset);
>  					} 
>  				}
>  				printf( "\n");
> @@ -8892,13 +8893,17 @@ dump_partials(void)
>  void
>  check_valid_partials(void)
>  {
> -        int num_partials[STREAM_MAX]; /* sum of partials for a given drive */
> +	int *num_partials; /* array for sum of partials for a given drive */
>  	partial_rest_t *isptr = NULL;
>  	bytespan_t *bsptr = NULL;
>  	int i;
>  
>  	/* zero the sums for each stream */
> -        memset(num_partials, 0, sizeof(num_partials));
> +	num_partials = calloc(drivecnt, sizeof(int));
> +	if (!num_partials) {
> +		perror("num_partials array allocation");
> +		return;
> +	}
>  
>  	pi_lock();
>  	if (persp->a.parrestcnt > 0) {
> @@ -8926,6 +8931,7 @@ check_valid_partials(void)
>  		}
>  	}
>  	pi_unlock();
> +	free(num_partials);
>  }
>  #endif
>  
> 
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

-- 
Carlos

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 1/2] xfsdump: avoid segfault in partial_reg() in error case

[Rich Johnston]

This has been committed.

Thanks
--Rich

commit 1162bdbcff77ed2341f0a9294db76df80f2f36a3
Author: Eric Sandeen <sandeen@sandeen.net>
Date:   Tue Oct 8 22:05:54 2013 +0000

     xfsdump: avoid segfault in partial_reg() in error case

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 2/2] xfsdump: fix DEBUGPARTIALS build

[Rich Johnston]

This has been commited.

Thanks
--Rich

commit 151858e9fdab6ebae490b05becc95f06aac2335c
Author: Eric Sandeen <sandeen@sandeen.net>
Date:   Tue Oct 8 22:31:41 2013 +0000

     xfsdump: fix DEBUGPARTIALS build

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs