From: Jeff King <peff@peff.net>
To: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Cc: "Matthieu Moy" <Matthieu.Moy@imag.fr>,
"René Scharfe" <l.s.r@web.de>, "Git List" <git@vger.kernel.org>,
"Junio C Hamano" <gitster@pobox.com>
Subject: [PATCH 1/8] config: fix bogus fd check when setting up default config
Date: Fri, 8 Jul 2016 05:06:50 -0400 [thread overview]
Message-ID: <20160708090649.GA10152@sigill.intra.peff.net> (raw)
In-Reply-To: <20160708090400.GA26594@sigill.intra.peff.net>
Since 9830534 (config --global --edit: create a template
file if needed, 2014-07-25), an edit of the global config
file will try to open() it with O_EXCL, and wants to handle
three cases:
1. We succeeded; the user has no config file, and we
should fill in the default template.
2. We got EEXIST; they have a file already, proceed as usual.
3. We got another error; we should complain.
However, the check for case 1 does "if (fd)", which will
generally _always_ be true (except for the oddball case that
somehow our stdin got closed and opening really did give us
a new descriptor 0).
So in the EEXIST case, we tried to write the default config
anyway! Fortunately, this turns out to be a noop, since we
just end up writing to and closing "-1", which does nothing.
But in case 3, we would fail to notice any other errors, and
just silently continue (given that we don't actually notice
write errors for the template either, it's probably not that
big a deal; we're about to spawn the editor, so it would
notice any problems. But the code is clearly _trying_ to hit
cover this case and failing).
We can fix it easily by using "fd >= 0" for case 1.
Signed-off-by: Jeff King <peff@peff.net>
---
I was looking at this to see whether it could be converted to
write_file(). However, the O_EXCL and the error-handling make things
too tricky.
builtin/config.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/config.c b/builtin/config.c
index 1d7c6ef..a991a53 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -604,7 +604,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)
given_config_source.file : git_path("config"));
if (use_global_config) {
int fd = open(config_file, O_CREAT | O_EXCL | O_WRONLY, 0666);
- if (fd) {
+ if (fd >= 0) {
char *content = default_user_config();
write_str_in_full(fd, content);
free(content);
--
2.9.0.393.g704e522
next prev parent reply other threads:[~2016-07-08 9:07 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-07-07 20:02 [PATCH] am: ignore return value of write_file() René Scharfe
2016-07-07 20:31 ` Jeff King
2016-07-08 6:37 ` Johannes Schindelin
2016-07-08 6:56 ` Jeff King
2016-07-08 9:04 ` [PATCH 0/8] write_file cleanups Jeff King
2016-07-08 9:06 ` Jeff King [this message]
2016-07-08 9:08 ` [PATCH 2/8] am: ignore return value of write_file() Jeff King
2016-07-08 9:08 ` [PATCH 3/8] branch: use non-gentle write_file for branch description Jeff King
2016-07-08 9:09 ` [PATCH 4/8] write_file: drop "gently" form Jeff King
2016-07-08 9:10 ` [PATCH 5/8] write_file: use xopen Jeff King
2016-07-08 9:12 ` [PATCH 6/8] write_file: add pointer+len variant Jeff King
2016-07-08 9:12 ` [PATCH 7/8] write_file: add format attribute Jeff King
2016-07-08 9:25 ` Jeff King
2016-07-08 9:25 ` [PATCH 1/2] walker: let walker_say take arbitrary formats Jeff King
2016-07-08 9:25 ` [PATCH 2/2] avoid using sha1_to_hex output as printf format Jeff King
2016-07-08 10:35 ` Jeff King
2016-07-08 17:02 ` Junio C Hamano
2016-07-08 17:09 ` Junio C Hamano
2016-07-08 21:41 ` Jeff King
2016-07-08 9:12 ` [PATCH 8/8] use write_file_buf where applicable Jeff King
2016-07-08 9:16 ` [PATCH 9/8] branch: use write_file_buf instead of write_file Jeff King
2016-07-08 18:44 ` [PATCH 0/8] write_file cleanups René Scharfe
2016-07-09 14:24 ` [PATCH] am: ignore return value of write_file() Johannes Schindelin
2016-07-10 10:53 ` Johannes Schindelin
2016-07-08 6:33 ` Johannes Schindelin
2016-07-08 18:44 ` René Scharfe
2016-07-08 21:51 ` Jeff King
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=20160708090649.GA10152@sigill.intra.peff.net \
--to=peff@peff.net \
--cc=Johannes.Schindelin@gmx.de \
--cc=Matthieu.Moy@imag.fr \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=l.s.r@web.de \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).