Re: [PATCH v2 2/4] fsck: do not die when not enough memory to examine a pack entry

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Jun 27, 2014 at 1:09 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Nguyễn Thái Ngọc Duy  <pclouds@xxxxxxxxx> writes:
>
>> fsck is a tool that error() is more preferred than die(), but many
>
> "more preferred" without justifying why it is "more preferred" is
> not quite a justification, is it?  Also, an object failing to load
> in-core is not a missing object, so if your aim is to let "fsck"
> diagnose a too-large-to-load object as missing and let it continue,
> I do not know if it is "more preferred" in the first place.  Adding
> a "too large--cannot check" bin of objects may be needed for it to
> be useful.  Also, we might need to give at the end "oh by the way,
> because we couldn't read some objects to even determine its type,
> the unreachable report from this fsck run is totally useless."

Fair enough. I think avoiding dying in xmalloc() in this code path is
still a good thing. At least "failed to read object %s" is more
informative than simply "Out of memory". The error cascading effect in
fsck is something I think we already have. I'll try to rephrase the
commit message. But if you think this is not a good direction,
dropping it is not so bad.

I'm going to look at xmalloc() in unpack-objects. That's where we
really should not abort because of memory shortage as the user may try
to get as many objects as possible out of the pack.

> The log message tries to justify that this may be a good thing for
> "fsck", but the patch actually tries to change the behaviour of all
> code paths that try to load an object in-core without considering
> the ramifications of such a change.  I _think_ all callers should be
> prepared to receive NULL when we encounter a corrupt object (and
> otherwise we should fix them), but it is unclear how much audit of
> the callers (if any) was done to prepare this change.
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]