Hi Junio,
On Mon, 22 Dec 2014, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@xxxxxx> 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.
> 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