Re: [PATCH 0/2] open() error checking
Junio C Hamano gits...@pobox.com writes:
Thomas Rast tr...@inf.ethz.ch writes:
I originally had a four-patch series to open 0/1/2 from /dev/null, but
then I noticed that this was shot down in 2008:
http://thread.gmane.org/gmane.comp.version-control.git/93605/focus=93896
The way I recall the thread was not shot down but more like
fizzled out without seeing a clear consensus. As a normal POSIX
program, we do rely on fd#2 connected to an error stream, and I do
agree with the general sentiment of that old thread that it is very
wrong for warning() or die() to write to a pipe or file descriptor
we opened for some other purpose, corrupting the destination.
I briefly wondered if we can do the sanity check lazily (e.g. upon
first warning() see of fd#2 is open and otherwise die silently), but
we may open a fd (e.g. to create a new loose object) that may happen
to grab fd#2 and then it is too late for us to do anything about it,
so...
I think we'd have to do it on startup. Since we do many things already,
a few extra dup calls should hardly matter.
I'll send the patches in reply in a minute, I had them lying around
already. But if you (again) decide that it's not worth it, I don't care
too deeply.
--
Thomas Rast
trast@{inf,student}.ethz.ch
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] open() error checking
Thomas Rast tr...@inf.ethz.ch writes: #1 is Dale's suggested change. Dale, to include it we'd need your Signed-off-by as per Documentation/SubmittingPatches. #2 is a similar error-checking fix; I reviewed 'git grep \bopen\b' and found one case where the return value was obviously not tested. The corresponding Windows code path has the same problem, but I dare not touch it; perhaps someone from the Windows side can look into it? I originally had a four-patch series to open 0/1/2 from /dev/null, but then I noticed that this was shot down in 2008: http://thread.gmane.org/gmane.comp.version-control.git/93605/focus=93896 The way I recall the thread was not shot down but more like fizzled out without seeing a clear consensus. As a normal POSIX program, we do rely on fd#2 connected to an error stream, and I do agree with the general sentiment of that old thread that it is very wrong for warning() or die() to write to a pipe or file descriptor we opened for some other purpose, corrupting the destination. I briefly wondered if we can do the sanity check lazily (e.g. upon first warning() see of fd#2 is open and otherwise die silently), but we may open a fd (e.g. to create a new loose object) that may happen to grab fd#2 and then it is too late for us to do anything about it, so... Do you want to resurrect this? The worst part about it is that because we don't have a stderr to rely on, we can't simply die(stop playing mind games). Right. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
