From: Eric Sandeen <sandeen@sandeen.net>
To: xfs@oss.sgi.com
Subject: Re: [PATCH 13/13] xfs_repair: Fix up warning strings in da_util.c
Date: Mon, 14 Sep 2015 15:11:23 -0500 [thread overview]
Message-ID: <55F729EB.2090605@sandeen.net> (raw)
In-Reply-To: <20150914200617.GM34083@bfoster.bfoster>
On 9/14/15 3:06 PM, Brian Foster wrote:
> On Wed, Sep 09, 2015 at 02:34:11PM -0500, Eric Sandeen wrote:
>> Switch the warning messages based on which fork has
>> encountered the problem.
>>
>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
>> ---
>
> I assume this is being reworked based on the discussion with Carlos (and
> I'm really not familiar with the localization stuff). That aside,
> shouldn't this be ordered at some point before these functions are
> called from both directory and attribute contexts so the messages aren't
> ever incorrect?
Well, tradeoffs I guess. I ordered it to make review easy. If we think
message correctness is a bisectability issue, I could rework it so that
it's always correct, but it didn't seem like that big a deal to me.
> Also, one nit below...
>
>> repair/attr_repair.c | 2 +-
>> repair/da_util.c | 97 +++++++++++++++++++++++++++-----------------------
>> repair/da_util.h | 3 +-
>> repair/dir2.c | 2 +-
>> 4 files changed, 56 insertions(+), 48 deletions(-)
>>
> ...
>> diff --git a/repair/da_util.c b/repair/da_util.c
>> index e5d5535..89d41cc 100644
>> --- a/repair/da_util.c
>> +++ b/repair/da_util.c
> ...
>> @@ -361,25 +366,27 @@ verify_final_da_path(
>> */
>> if (cursor->level[this_level].hashval >=
>> be32_to_cpu(btree[entry].hashval)) {
>> - do_warn(_("directory/attribute block hashvalue inconsistency, "
>> - "expected > %u / saw %u\n"),
>> + do_warn(
>> +_("%s block hashvalue inconsistency, expected > %u / saw %u\n"),
>> + FORKNAME(whichfork),
>> cursor->level[this_level].hashval,
>> be32_to_cpu(btree[entry].hashval));
>> bad++;
>> }
>> if (nodehdr.forw != 0) {
>> - do_warn(_("bad directory/attribute forward block pointer, "
>> - "expected 0, saw %u\n"),
>> - nodehdr.forw);
>> + do_warn(
>> +_("bad %s forward block pointer, expected 0, saw %u\n"),
>> + FORKNAME(whichfork), nodehdr.forw);
>> bad++;
>> }
>> if (bad) {
>> - do_warn(_("bad directory block in inode %" PRIu64 "\n"), cursor->ino);
>> + do_warn(_("bad %s block in inode %" PRIu64 "\n"),
>> + FORKNAME(whichfork), cursor->ino);
>> return 1;
>> }
>> /*
>> * keep track of greatest block # -- that gets
>> - * us the length of the directory
>> + * us the length of the directory/attribute
>
> Trailing whitespace above.
ok :)
Thanks,
-Eric
> Brian
>
>> */
>> if (cursor->level[this_level].bno > cursor->greatest_bno)
>> cursor->greatest_bno = cursor->level[this_level].bno;
>> @@ -389,9 +396,9 @@ verify_final_da_path(
>> */
>> if (cursor->level[p_level].bno != be32_to_cpu(btree[entry].before)) {
>> #ifdef XR_DIR_TRACE
>> - fprintf(stderr, "bad directory btree pointer, child bno should "
>> + fprintf(stderr, "bad %s btree pointer, child bno should "
>> "be %d, block bno is %d, hashval is %u\n",
>> - be16_to_cpu(btree[entry].before),
>> + FORKNAME(whichfork), be16_to_cpu(btree[entry].before),
>> cursor->level[p_level].bno,
>> cursor->level[p_level].hashval);
>> fprintf(stderr, "verify_final_da_path returns 1 (bad) #1a\n");
>> @@ -403,17 +410,17 @@ verify_final_da_path(
>> be32_to_cpu(btree[entry].hashval)) {
>> if (!no_modify) {
>> do_warn(
>> -_("correcting bad hashval in non-leaf dir block\n"
>> +_("correcting bad hashval in non-leaf %s block\n"
>> "\tin (level %d) in inode %" PRIu64 ".\n"),
>> - this_level, cursor->ino);
>> + FORKNAME(whichfork), this_level, cursor->ino);
>> btree[entry].hashval = cpu_to_be32(
>> cursor->level[p_level].hashval);
>> cursor->level[this_level].dirty++;
>> } else {
>> do_warn(
>> -_("would correct bad hashval in non-leaf dir block\n"
>> +_("would correct bad hashval in non-leaf %s block\n"
>> "\tin (level %d) in inode %" PRIu64 ".\n"),
>> - this_level, cursor->ino);
>> + FORKNAME(whichfork), this_level, cursor->ino);
>> }
>> }
>>
>> @@ -451,7 +458,7 @@ _("would correct bad hashval in non-leaf dir block\n"
>> */
>> cursor->level[this_level].hashval = hashval;
>>
>> - return verify_final_da_path(mp, cursor, this_level);
>> + return verify_final_da_path(mp, cursor, this_level, whichfork);
>> }
>>
>> /*
>> @@ -564,8 +571,8 @@ verify_da_path(
>> &bmp, &lbmp);
>> if (nex == 0) {
>> do_warn(
>> -_("can't get map info for block %u of directory inode %" PRIu64 "\n"),
>> - dabno, cursor->ino);
>> +_("can't get map info for %s block %u of inode %" PRIu64 "\n"),
>> + FORKNAME(whichfork), dabno, cursor->ino);
>> return 1;
>> }
>>
>> @@ -575,8 +582,8 @@ _("can't get map info for block %u of directory inode %" PRIu64 "\n"),
>>
>> if (!bp) {
>> do_warn(
>> -_("can't read block %u for directory inode %" PRIu64 "\n"),
>> - dabno, cursor->ino);
>> +_("can't read %s block %u for inode %" PRIu64 "\n"),
>> + FORKNAME(whichfork), dabno, cursor->ino);
>> return 1;
>> }
>>
>> @@ -592,28 +599,28 @@ _("can't read block %u for directory inode %" PRIu64 "\n"),
>> if (nodehdr.magic != XFS_DA_NODE_MAGIC &&
>> nodehdr.magic != XFS_DA3_NODE_MAGIC) {
>> do_warn(
>> -_("bad magic number %x in block %u for directory inode %" PRIu64 "\n"),
>> - nodehdr.magic,
>> +_("bad magic number %x in %s block %u for inode %" PRIu64 "\n"),
>> + nodehdr.magic, FORKNAME(whichfork),
>> dabno, cursor->ino);
>> bad++;
>> }
>> if (nodehdr.back != cursor->level[this_level].bno) {
>> do_warn(
>> -_("bad back pointer in block %u for directory inode %" PRIu64 "\n"),
>> - dabno, cursor->ino);
>> +_("bad back pointer in %s block %u for inode %" PRIu64 "\n"),
>> + FORKNAME(whichfork), dabno, cursor->ino);
>> bad++;
>> }
>> if (nodehdr.count > geo->node_ents) {
>> do_warn(
>> -_("entry count %d too large in block %u for directory inode %" PRIu64 "\n"),
>> - nodehdr.count,
>> +_("entry count %d too large in %s block %u for inode %" PRIu64 "\n"),
>> + nodehdr.count, FORKNAME(whichfork),
>> dabno, cursor->ino);
>> bad++;
>> }
>> if (nodehdr.level != this_level) {
>> do_warn(
>> -_("bad level %d in block %u for directory inode %" PRIu64 "\n"),
>> - nodehdr.level,
>> +_("bad level %d in %s block %u for inode %" PRIu64 "\n"),
>> + nodehdr.level, FORKNAME(whichfork),
>> dabno, cursor->ino);
>> bad++;
>> }
>> @@ -659,9 +666,9 @@ _("bad level %d in block %u for directory inode %" PRIu64 "\n"),
>> */
>> if (cursor->level[p_level].bno != be32_to_cpu(btree[entry].before)) {
>> #ifdef XR_DIR_TRACE
>> - fprintf(stderr, "bad directory btree pointer, child bno "
>> + fprintf(stderr, "bad %s btree pointer, child bno "
>> "should be %d, block bno is %d, hashval is %u\n",
>> - be32_to_cpu(btree[entry].before),
>> + FORKNAME(whichfork), be32_to_cpu(btree[entry].before),
>> cursor->level[p_level].bno,
>> cursor->level[p_level].hashval);
>> fprintf(stderr, "verify_da_path returns 1 (bad) #1a\n");
>> @@ -676,17 +683,17 @@ _("bad level %d in block %u for directory inode %" PRIu64 "\n"),
>> be32_to_cpu(btree[entry].hashval)) {
>> if (!no_modify) {
>> do_warn(
>> -_("correcting bad hashval in interior dir block\n"
>> +_("correcting bad hashval in interior %s block\n"
>> "\tin (level %d) in inode %" PRIu64 ".\n"),
>> - this_level, cursor->ino);
>> + FORKNAME(whichfork), this_level, cursor->ino);
>> btree[entry].hashval = cpu_to_be32(
>> cursor->level[p_level].hashval);
>> cursor->level[this_level].dirty++;
>> } else {
>> do_warn(
>> -_("would correct bad hashval in interior dir block\n"
>> +_("would correct bad hashval in interior %s block\n"
>> "\tin (level %d) in inode %" PRIu64 ".\n"),
>> - this_level, cursor->ino);
>> + FORKNAME(whichfork), this_level, cursor->ino);
>> }
>> }
>> /*
>> diff --git a/repair/da_util.h b/repair/da_util.h
>> index 7971d63..442f9f1 100644
>> --- a/repair/da_util.h
>> +++ b/repair/da_util.h
>> @@ -78,5 +78,6 @@ int
>> verify_final_da_path(
>> xfs_mount_t *mp,
>> da_bt_cursor_t *cursor,
>> - const int p_level);
>> + const int p_level,
>> + int whichfork);
>> #endif /* _XR_DA_UTIL_H */
>> diff --git a/repair/dir2.c b/repair/dir2.c
>> index 492b3e7..61912d1 100644
>> --- a/repair/dir2.c
>> +++ b/repair/dir2.c
>> @@ -1179,7 +1179,7 @@ _("bad sibling back pointer for block %u in directory inode %" PRIu64 "\n"),
>> } else
>> libxfs_putbuf(bp);
>> } while (da_bno != 0);
>> - if (verify_final_da_path(mp, da_cursor, 0)) {
>> + if (verify_final_da_path(mp, da_cursor, 0, XFS_DATA_FORK)) {
>> /*
>> * Verify the final path up (right-hand-side) if still ok.
>> */
>> --
>> 1.7.1
>>
>> _______________________________________________
>> xfs mailing list
>> xfs@oss.sgi.com
>> http://oss.sgi.com/mailman/listinfo/xfs
>
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs
>
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
next prev parent reply other threads:[~2015-09-14 20:11 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-09-09 19:33 [PATCH 0/13] xfs_repair: recombine cut&waste code in dir2.c/attr_repair.c Eric Sandeen
2015-09-09 19:33 ` [PATCH 01/13] xfs_repair: remove trace-only 'n' member from da_level_state Eric Sandeen
2015-09-14 19:17 ` Brian Foster
2015-09-09 19:34 ` [PATCH 02/13] xfs_repair: remove type from da & dir2 cursors Eric Sandeen
2015-09-14 19:18 ` Brian Foster
2015-09-09 19:34 ` [PATCH 03/13] xfs_repair: make CRC checking consistent in path verification Eric Sandeen
2015-09-14 19:18 ` Brian Foster
2015-09-09 19:34 ` [PATCH 04/13] xfs_repair: use multibuffer read routines in attr_repair.c Eric Sandeen
2015-09-14 19:18 ` Brian Foster
2015-09-14 19:24 ` Eric Sandeen
2015-09-14 19:30 ` Brian Foster
2015-09-09 19:34 ` [PATCH 05/13] xfs_repair: fix use-after-free in verify_final_dir2_path Eric Sandeen
2015-09-14 19:18 ` Brian Foster
2015-09-09 19:34 ` [PATCH 06/13] xfs_repair: add XR_DIR_TRACE to dir2.c Eric Sandeen
2015-09-14 19:18 ` Brian Foster
2015-09-09 19:34 ` [PATCH 07/13] xfs_repair: Remove BUF_PTR from attr_repair.c Eric Sandeen
2015-09-14 19:44 ` Brian Foster
2015-09-09 19:34 ` [PATCH 08/13] xfs_repair: catch bad level/depth in da node Eric Sandeen
2015-09-14 19:44 ` Brian Foster
2015-09-09 19:34 ` [PATCH 09/13] xfs_repair: better checking of v5 attributes Eric Sandeen
2015-09-14 19:44 ` Brian Foster
2015-09-23 17:53 ` Eric Sandeen
2015-09-09 19:34 ` [PATCH 10/13] xfs_repair: Remove more differences between attr & dir2 Eric Sandeen
2015-09-14 19:55 ` Brian Foster
2015-09-09 19:34 ` [PATCH 11/13] xfs_repair: whitespace & comments Eric Sandeen
2015-09-14 19:56 ` Brian Foster
2015-09-09 19:34 ` [PATCH 12/13] xfs_repair: move common dir2 and attr_repair code to da_util.c Eric Sandeen
2015-09-09 19:34 ` [PATCH 13/13] xfs_repair: Fix up warning strings in da_util.c Eric Sandeen
2015-09-14 20:06 ` Brian Foster
2015-09-14 20:11 ` Eric Sandeen [this message]
2015-09-10 9:22 ` [PATCH 0/13] xfs_repair: recombine cut&waste code in dir2.c/attr_repair.c Carlos Maiolino
2015-09-10 16:51 ` Eric Sandeen
2015-09-11 8:20 ` Carlos Maiolino
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=55F729EB.2090605@sandeen.net \
--to=sandeen@sandeen.net \
--cc=xfs@oss.sgi.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox