| Message ID | 20180509114554.2slvfi3hqt4na6vy@odin.usersys.redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, May 09, 2018 at 01:45:54PM +0200, Carlos Maiolino wrote: > Hey folks, > > I've been thinking about different ways to fix those crashes on xfs_db due type > mismatch, based on the previous discussion and a chat I had with Eric on irc. > > After thinking a bit more about it, I do think using unknown header types adds > unneeded extra complexity to the code, to achieve the same result as an extra > check on print_flist_1(). > > Adding an unknown type, will require a change to all types affected by this > issue. I am not sure if a single unknown type can be used for all metadata types > which is affected by this, but I think it requires an unknown type for each of > these metadata. Or at least an entry for an unknown header type (in case of of > the known ones are not found). > > While, the crash caused by unrecognized metadata when using these types, can be > fixed by something like my original change. > > I wrote a slightly different patch like this: > > diff --git a/db/print.c b/db/print.c > index 0da36c27..f8a48281 100644 > --- a/db/print.c > +++ b/db/print.c > @@ -160,9 +160,10 @@ print_flist_1( > (f->flags & FLD_ARRAY) != 0); > if (neednl) > dbprintf("\n"); > - } else { > - ASSERT(fa->arg & FTARG_OKEMPTY); > + } else if (fa->arg & FTARG_OKEMPTY) { > dbprintf(_("(empty)\n")); > + } else { > + dbprintf(_("Unrecognized metadata or type mismatch\n")); > } > } > free_strvec(pfx); > > > This will still keep printing "(empty)" where it should be printed, or print out > a hint that something went wrong. > > I honestly think this the best solution for these crashes, what you guys think? Looks fine to me, I dislike ASSERTs on crap metadata, especially in the debugging tool. Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> --D > > Cheers > > > -- > Carlos > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 5/9/18 6:45 AM, Carlos Maiolino wrote: > Hey folks, > > I've been thinking about different ways to fix those crashes on xfs_db due type > mismatch, based on the previous discussion and a chat I had with Eric on irc. > > After thinking a bit more about it, I do think using unknown header types adds > unneeded extra complexity to the code, to achieve the same result as an extra > check on print_flist_1(). > > Adding an unknown type, will require a change to all types affected by this > issue. I am not sure if a single unknown type can be used for all metadata types > which is affected by this, but I think it requires an unknown type for each of > these metadata. Or at least an entry for an unknown header type (in case of of > the known ones are not found). > > While, the crash caused by unrecognized metadata when using these types, can be > fixed by something like my original change. > > I wrote a slightly different patch like this: Ok, I'll merge this, with a proper commit log. > diff --git a/db/print.c b/db/print.c > index 0da36c27..f8a48281 100644 > --- a/db/print.c > +++ b/db/print.c > @@ -160,9 +160,10 @@ print_flist_1( > (f->flags & FLD_ARRAY) != 0); > if (neednl) > dbprintf("\n"); > - } else { > - ASSERT(fa->arg & FTARG_OKEMPTY); > + } else if (fa->arg & FTARG_OKEMPTY) { > dbprintf(_("(empty)\n")); > + } else { > + dbprintf(_("Unrecognized metadata or type mismatch\n")); > } > } > free_strvec(pfx); > > > This will still keep printing "(empty)" where it should be printed, or print out > a hint that something went wrong. > > I honestly think this the best solution for these crashes, what you guys think? > > Cheers > > -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/db/print.c b/db/print.c index 0da36c27..f8a48281 100644 --- a/db/print.c +++ b/db/print.c @@ -160,9 +160,10 @@ print_flist_1( (f->flags & FLD_ARRAY) != 0); if (neednl) dbprintf("\n"); - } else { - ASSERT(fa->arg & FTARG_OKEMPTY); + } else if (fa->arg & FTARG_OKEMPTY) { dbprintf(_("(empty)\n")); + } else { + dbprintf(_("Unrecognized metadata or type mismatch\n")); } } free_strvec(pfx);