linux-ext4.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andreas Dilger <adilger@dilger.ca>
To: Jan Kara <jack@suse.cz>
Cc: Ted Tso <tytso@mit.edu>, linux-ext4@vger.kernel.org
Subject: Re: [PATCH 1/3] e2fsck: Clarify overflow link count error message
Date: Wed, 5 Feb 2020 10:38:04 -0700	[thread overview]
Message-ID: <23FB58CC-BE93-4602-8B1C-9DA06FAE0F1A@dilger.ca> (raw)
In-Reply-To: <20200205100138.30053-2-jack@suse.cz>

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

On Feb 5, 2020, at 3:01 AM, Jan Kara <jack@suse.cz> wrote:
> 
> When directory link count is set to overflow value (1) but during pass 4
> we find out the exact link count would fit, we either silently fix this
> (which is not great because e2fsck then reports the fs was modified but
> output doesn't indicate why in any way), or we report that link count is
> wrong and ask whether we should fix it (in case -n option was
> specified). The second case is even more misleading because it suggests
> non-trivial fs corruption which then gets silently fixed on the next
> run. Similarly to how we fix up other non-problems, just create a new
> error message for the case directory link count is not overflown anymore
> and always report it to clarify what is going on.
> 
> 
> diff --git a/e2fsck/problem.c b/e2fsck/problem.c
> index c7c0ba986006..cde369d03034 100644
> --- a/e2fsck/problem.c
> +++ b/e2fsck/problem.c
> @@ -2035,6 +2035,11 @@ static struct e2fsck_problem problem_table[] = {
> 	  N_("@d exceeds max links, but no DIR_NLINK feature in @S.\n"),
> 	  PROMPT_FIX, 0, 0, 0, 0 },
> 
> +	/* Directory ref count set to overflow but it doesn't have to be */

> +	{ PR_4_DIR_OVERFLOW_REF_COUNT,
> +	  N_("@d @i %i ref count set to overflow value %Il but could be exact value %N.  "),

IMHO, you don't need to print "value %Il" since that will always be "1"?
That would shorten the message to fit on a single line.

Also, lease keep the comment and the actual error message identical.
Otherwise, it is harder to find the PR_* number and the related code in
e2fsck when trying to debug a problem.  So the comment should be:

	/* Directory inode ref count set to overflow but could be exact value */

To be honest, I don't see the benefit of the @d, @i, etc. abbreviations
in the messages anymore.  The few bytes of space savings is IMHO outweighed
by the added complexity in understanding and finding the messages in the
code, and added complexity in e2fsck itself when printing the messages.


Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]

  reply	other threads:[~2020-02-05 17:38 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-05 10:01 [PATCH 0/3] e2fsprogs: Better handling of indexed directories Jan Kara
2020-02-05 10:01 ` [PATCH 1/3] e2fsck: Clarify overflow link count error message Jan Kara
2020-02-05 17:38   ` Andreas Dilger [this message]
2020-02-06  9:59     ` Jan Kara
2020-02-05 10:01 ` [PATCH 2/3] ext2fs: Implement dir entry creation in htree directories Jan Kara
2020-02-05 17:50   ` Andreas Dilger
2020-02-06 10:07     ` Jan Kara
2020-02-05 10:01 ` [PATCH 3/3] tests: Add tests for ext2fs_link() into htree dir Jan Kara
2020-02-05 18:11   ` Andreas Dilger
2020-02-06 10:16     ` Jan Kara
2020-02-06 23:04       ` Andreas Dilger
2020-02-10  9:18         ` Jan Kara
2020-02-05 15:24 ` [PATCH 0/3] e2fsprogs: Better handling of indexed directories Jan Kara

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=23FB58CC-BE93-4602-8B1C-9DA06FAE0F1A@dilger.ca \
    --to=adilger@dilger.ca \
    --cc=jack@suse.cz \
    --cc=linux-ext4@vger.kernel.org \
    --cc=tytso@mit.edu \
    /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;
as well as URLs for NNTP newsgroup(s).