Re: [PATCH] quickfetch(): Prevent overflow of the rev-list command line
- From
Johan Herland <johan@herland.net>
- Date
- Jul 9, 2009, 08:37 UTC
- Message-ID
- <200907091037.41329.johan@herland.net>
- In-Reply-To
- <81b0412b0907090101x7c8aa182o36687d67be3c5fb0@mail.gmail.com>
On Thursday 09 July 2009, Alex Riesen wrote:
Show 17 quoted lines
> On Wed, Jul 8, 2009 at 18:01, Johan Herland<johan@herland.net> wrote:
> > On Wednesday 08 July 2009, Johannes Sixt wrote:
> >> ... don't you get this error message with errno set to EPIPE?
> >> Previously, there was no error message.
> >
> > Indeed, you are correct. I guess the following should be added to the
> > patch:
> >
> > if (write_in_full(revlist.in, sha1_to_hex(ref->old_sha1), 40) <
> > 0 || write_in_full(revlist.in, "\n", 1) < 0) {
> > - error("failed write to rev-list");
> > - err = errno;
> > + if (errno != EPIPE) {
> > + error("failed write to rev-list");
> > + err = errno;
>
> You'll loose errno this way: error() does not save it.Not sure what you mean here. Should I move "err = errno;" outside the innermost "if"?
>From my POV, if errno != EPIPE, we save it into err, and return that
(overridden by finish_command()'s return value, if non-zero). If errno == EPIPE, we're not interested in saving it, because we expect finish_command() to return non-zero in any case.
...Johan
-- Johan Herland, <johan@herland.net> www.herland.net