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

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

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

From: Junio C Hamano <hidden>
Date: 2016-06-15 23:03:20

Johannes Schindelin [off-list ref] writes:
quoted
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.

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.

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

From: Johannes Schindelin <hidden>
Date: 2016-06-15 23:03:20

Hi Junio,

On Mon, 22 Dec 2014, Junio C Hamano wrote:
Johannes Schindelin [off-list ref] writes:
quoted
quoted
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help