Re: [PATCH 04/18] Offer a function to demote fsck errors to warnings

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Dec 22, 2014, 22:32 UTC
Message-ID: <alpine.DEB.1.00.1412222330080.21312@s15462909.onlinehome-server.info>
In-Reply-To: <xmqqfvc78hwq.fsf@gitster.dls.corp.google.com>

Hi Junio,

On Mon, 22 Dec 2014, Junio C Hamano wrote:

Show 25 quoted lines

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> >> In other words, at some point wouldn't we be better off with
> >> something like this
> >> 
> >> 	struct {
> >>         	enum id;
> >>                 const char *id_string;
> >>                 enum error_level { FSCK_PASS, FSCK_WARN, FSCK_ERROR };
> >> 	} possible_fsck_errors[];
> >
> > I considered that, and Michael Haggerty also suggested that in a private
> > mail. However, I find that there is a clear hierarchy in the default
> > messages: fatal errors, errors, warnings and infos.
> 
> I am glad I am not alone ;-)
> 
> These classes are ordered from more severe to less, but I do not
> think it makes much sense to force the default view of "if you
> customize to demote a questionable Q that is classified as an error
> by default as an warning, you must demote all the other ones that we
> deem less serious than Q, which come earlier (or later---I do not
> remember which) in our predefined list".  So in that sense, I do not
> consider that various kinds of questionables fsck can detect are
> hierarchical at all.

Oh, but please understand that this hierarchy only applies to the default settings. All of these settings can be overridden individually – and the first override will initialize a full array with the default settings.

So the order really only plays a role for the defaults, no more.

Show 14 quoted lines

> I do agree that it makes it easier to code the initialization of
> such an array to have "up to this point we assign the level 'fatal
> error' by default" constants.  Then the initialization can become
> 
> 	for (i = 0; i < FIRST_WARN; i++)
>         	possible_fsck_errors[i].error_level = FSCK_INFO;
> 	while (i < FIRST_ERROR)
>         	possible_fsck_errors[i++].error_level = FSCK_WARN;
> 	while (i < ARRAY_SIZE(possible_fsck_errors))
>         	possible_fsck_errors[i++].error_level = FSCK_ERROR;
> 
> or something.  So I am not against the FIRST_WARNING constant at
> all, but I find it very questionable in a fully customizable system
> to use such a constant anywhere other than the initialization time.

This is indeed the case. The code we are discussing comes after the

	if (options->strict_mode)
		return options->strict_mode[msg_id];

In other words, once the overrides are in place, the default settings are skipped entirely.

Ciao, Dscho