All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jonathan Nieder <jrnieder@gmail.com>
To: Jeff King <peff@peff.net>
Cc: git@vger.kernel.org, Michael Haggerty <mhagger@alum.mit.edu>
Subject: Re: [PATCH 2/2] read_info_alternates: warn on non-trivial errors
Date: Mon, 18 Sep 2017 19:46:24 -0700	[thread overview]
Message-ID: <20170919024623.GD175206@aiede.mtv.corp.google.com> (raw)
In-Reply-To: <20170918155519.llmetd2ylebzokrl@sigill.intra.peff.net>

Jeff King wrote:

> When we fail to open $GIT_DIR/info/alternates, we silently
> assume there are no alternates. This is the right thing to
> do for ENOENT, but not for other errors.
>
> A hard error is probably overkill here. If we fail to read
> an alternates file then either we'll complete our operation
> anyway, or we'll fail to find some needed object. Either
> way, a warning is good idea. And we already have a helper
> function to handle this pattern; let's just call
> warn_on_fopen_error().

I think I prefer a hard error.  What kind of cases are you imagining
where it would be better to warn?

E.g. for EIO, erroring out so that the user can try again seems better
than hoping that the application will be able to cope with the more
subtle error that comes from discovering some objects are missing.

For EACCES, I can see how it makes sense to warn and move on, but no
other errors like that are occuring to me.

Thoughts?

Thanks,
Jonathan

> Note that technically the errno from strbuf_read_file()
> might be from a read() error, not open(). But since read()
> would never return ENOENT or ENOTDIR, and since it produces
> a generic "unable to access" error, it's suitable for
> handling errors from either.

  reply	other threads:[~2017-09-19  2:46 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-18 15:51 [PATCH 0/2] fix read past end of array in alternates files Jeff King
2017-09-18 15:54 ` [PATCH 1/2] read_info_alternates: read contents into strbuf Jeff King
2017-09-19  0:08   ` Junio C Hamano
2017-09-19  2:42   ` Jonathan Nieder
2017-09-19  5:03     ` Jeff King
2017-09-18 15:55 ` [PATCH 2/2] read_info_alternates: warn on non-trivial errors Jeff King
2017-09-19  2:46   ` Jonathan Nieder [this message]
2017-09-19  5:15     ` Jeff King
2017-09-19  2:36 ` [PATCH 0/2] fix read past end of array in alternates files Jonathan Nieder
2017-09-19  4:57   ` Jeff King
2017-09-19 19:40 ` [PATCH v2 " Jeff King
2017-09-19 19:41   ` [PATCH v2 1/2] read_info_alternates: read contents into strbuf Jeff King
2017-09-19 19:41   ` [PATCH v2 2/2] read_info_alternates: warn on non-trivial errors Jeff King
2017-09-20  2:44   ` [PATCH v2 0/2] fix read past end of array in alternates files Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170919024623.GD175206@aiede.mtv.corp.google.com \
    --to=jrnieder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=mhagger@alum.mit.edu \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.