git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 1/8] config: fix bogus fd check when setting up default config

From
Jeff King <peff@peff.net>
Date
Jul 8, 2016, 09:06 UTC
Message-ID
<20160708090649.GA10152@sigill.intra.peff.net>
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
Previous: Jeff KingNext: Jeff King
Message 6 of 27 in “am: ignore return value of write_file()”
  1. am: ignore return value of write_file()René Scharfe, Jul 7, 2016
  2. Jeff KingJul 7, 2016
  3. Johannes SchindelinJul 8, 2016
  4. Jeff KingJul 8, 2016
  5. 0/8 write_file cleanupsJeff King, Jul 8, 2016
  6. 1/8 config: fix bogus fd check when setting up default configJeff King, Jul 8, 2016
  7. 2/8 am: ignore return value of write_file()Jeff King, Jul 8, 2016
  8. 3/8 branch: use non-gentle write_file for branch descriptionJeff King, Jul 8, 2016
  9. 4/8 write_file: drop "gently" formJeff King, Jul 8, 2016
  10. 5/8 write_file: use xopenJeff King, Jul 8, 2016
  11. 6/8 write_file: add pointer+len variantJeff King, Jul 8, 2016
  12. 7/8 write_file: add format attributeJeff King, Jul 8, 2016
  13. Jeff KingJul 8, 2016
  14. 1/2 walker: let walker_say take arbitrary formatsJeff King, Jul 8, 2016
  15. 2/2 avoid using sha1_to_hex output as printf formatJeff King, Jul 8, 2016
  16. Jeff KingJul 8, 2016
  17. Junio C HamanoJul 8, 2016
  18. Junio C HamanoJul 8, 2016
  19. Jeff KingJul 8, 2016
  20. 8/8 use write_file_buf where applicableJeff King, Jul 8, 2016
  21. 9/8 branch: use write_file_buf instead of write_fileJeff King, Jul 8, 2016
  22. René ScharfeJul 8, 2016
  23. Johannes SchindelinJul 9, 2016
  24. Johannes SchindelinJul 10, 2016
  25. Johannes SchindelinJul 8, 2016
  26. René ScharfeJul 8, 2016
  27. Jeff KingJul 8, 2016

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.