Re: [PATCH] fsck: check skiplist for object in fsck_blob()

From: Ramsay Jones <ramsay@ramsayjones.plus.com>
Date: Jun 29, 2018, 01:10 UTC
Message-ID: <9162ed69-d245-8b2f-0dcc-3b345264b029@ramsayjones.plus.com>
In-Reply-To: <20180628220332.GA5128@sigill.intra.peff.net>

On 28/06/18 23:03, Jeff King wrote:

> On Thu, Jun 28, 2018 at 07:53:27PM +0100, Ramsay Jones wrote:

[snip]

> Yes, it can go in quickly. But I'd prefer not to keep it in the long
> term if it's literally doing nothing.

Hmm, I don't think you can say its doing nothing!

    "Yeah, this solution seems sensible. Given that we would
     never report any error for that blob, there is no point
     in even looking at it."

... is no less true, with or without additional patches! ;-)

> I have some patches which I think solve your problem. They apply on
> v2.18.0, but not on v2.17.1 (because they rely on Dscho's increased
> passing of config_options in v2.18). Is that good enough?

Heh, I was also writing patches to address this tonight (but I was also watching the football, so I was somewhat behind you). My patches were not too dissimilar to yours, except I was aiming to allow even do_config_from_file() etc., to suppress errors.

Your patches were cleaner and more focused than mine. (Instead of turning die_on_error into an enum, I added an additional 'quiet' flag. When pushing the stack (eg. for include files), I had to copy the quiet flag from the parent struct, etc, ... ;-) ).

> Yes, it would include any syntax error. I also have a slight worry about
> that, but nobody seems to have screamed _yet_. :)

Hmm, I don't think we can ignore this. :(

> Here are the patches I came up with.

Yes, I applied these locally and tested them. All OK here.

So, FWIW, Ack!

[I still think my original patch, with the 'to_be_skipped' function name changed to 'object_on_skiplist', should be the first patch of the series!]

ATB, Ramsay Jones