Re: [PATCH 1/3] e2fsck: Clarify overflow link count error message

[Jan Kara <jack@suse.cz>]

On Wed 05-02-20 10:38:04, Andreas Dilger wrote:
> 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.

Yeah, will change.

> 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 */

Sure, thanks for catching this.

> 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.

I tend to agree but I was never bothered enough to try to change that.

									Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR