* [PATCH 0/2] xfsdump: 2 more fixes @ 2013-10-08 22:01 Eric Sandeen 2013-10-08 22:05 ` [PATCH 1/2] xfsdump: avoid segfault in partial_reg() in error case Eric Sandeen 2013-10-08 22:31 ` [PATCH 2/2] xfsdump: fix DEBUGPARTIALS build Eric Sandeen 0 siblings, 2 replies; 7+ messages in thread From: Eric Sandeen @ 2013-10-08 22:01 UTC (permalink / raw) To: xfs-oss 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] xfsdump: avoid segfault in partial_reg() in error case 2013-10-08 22:01 [PATCH 0/2] xfsdump: 2 more fixes Eric Sandeen @ 2013-10-08 22:05 ` Eric Sandeen 2013-10-10 14:17 ` Carlos Maiolino 2013-10-18 21:29 ` Rich Johnston 2013-10-08 22:31 ` [PATCH 2/2] xfsdump: fix DEBUGPARTIALS build Eric Sandeen 1 sibling, 2 replies; 7+ messages in thread From: Eric Sandeen @ 2013-10-08 22:05 UTC (permalink / raw) To: xfs-oss 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] xfsdump: avoid segfault in partial_reg() in error case 2013-10-08 22:05 ` [PATCH 1/2] xfsdump: avoid segfault in partial_reg() in error case Eric Sandeen @ 2013-10-10 14:17 ` Carlos Maiolino 2013-10-18 21:29 ` Rich Johnston 1 sibling, 0 replies; 7+ messages in thread From: Carlos Maiolino @ 2013-10-10 14:17 UTC (permalink / raw) To: xfs 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] xfsdump: avoid segfault in partial_reg() in error case 2013-10-08 22:05 ` [PATCH 1/2] xfsdump: avoid segfault in partial_reg() in error case Eric Sandeen 2013-10-10 14:17 ` Carlos Maiolino @ 2013-10-18 21:29 ` Rich Johnston 1 sibling, 0 replies; 7+ messages in thread From: Rich Johnston @ 2013-10-18 21:29 UTC (permalink / raw) To: Eric Sandeen, xfs-oss 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] xfsdump: fix DEBUGPARTIALS build 2013-10-08 22:01 [PATCH 0/2] xfsdump: 2 more fixes Eric Sandeen 2013-10-08 22:05 ` [PATCH 1/2] xfsdump: avoid segfault in partial_reg() in error case Eric Sandeen @ 2013-10-08 22:31 ` Eric Sandeen 2013-10-10 14:21 ` Carlos Maiolino 2013-10-18 21:30 ` Rich Johnston 1 sibling, 2 replies; 7+ messages in thread From: Eric Sandeen @ 2013-10-08 22:31 UTC (permalink / raw) To: xfs-oss 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] xfsdump: fix DEBUGPARTIALS build 2013-10-08 22:31 ` [PATCH 2/2] xfsdump: fix DEBUGPARTIALS build Eric Sandeen @ 2013-10-10 14:21 ` Carlos Maiolino 2013-10-18 21:30 ` Rich Johnston 1 sibling, 0 replies; 7+ messages in thread From: Carlos Maiolino @ 2013-10-10 14:21 UTC (permalink / raw) To: xfs 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] xfsdump: fix DEBUGPARTIALS build 2013-10-08 22:31 ` [PATCH 2/2] xfsdump: fix DEBUGPARTIALS build Eric Sandeen 2013-10-10 14:21 ` Carlos Maiolino @ 2013-10-18 21:30 ` Rich Johnston 1 sibling, 0 replies; 7+ messages in thread From: Rich Johnston @ 2013-10-18 21:30 UTC (permalink / raw) To: Eric Sandeen, xfs-oss 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-10-18 21:30 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-10-08 22:01 [PATCH 0/2] xfsdump: 2 more fixes Eric Sandeen 2013-10-08 22:05 ` [PATCH 1/2] xfsdump: avoid segfault in partial_reg() in error case Eric Sandeen 2013-10-10 14:17 ` Carlos Maiolino 2013-10-18 21:29 ` Rich Johnston 2013-10-08 22:31 ` [PATCH 2/2] xfsdump: fix DEBUGPARTIALS build Eric Sandeen 2013-10-10 14:21 ` Carlos Maiolino 2013-10-18 21:30 ` Rich Johnston
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox