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

Re: [PATCHv4] Read (but not write) from XDG configuration, XDG attributes and XDG ignore files

From
Ramsay Jones <ramsay@ramsay1.demon.co.uk>
Date
Jun 4, 2012, 17:54 UTC
Message-ID
<4FCCF63B.8070609@ramsay1.demon.co.uk>
In-Reply-To
<1338585788-9764-1-git-send-email-Huynh-Khoi-Nguyen.Nguyen@ensimag.imag.fr>
Huynh Khoi Nguyen NGUYEN wrote:
Show 30 quoted lines
> From: NGUYEN Huynh Khoi Nguyen <nguyenhu@ensibm.imag.fr>
> 
> Git will be able to read in $XDG_CONFIG_HOME/git/config, a new
> configuration file following XDG specification. In the order of
> reading, this file is between global configuration file and system
> wide configuration file. Git will not be able to write in this new
> configuration file. If core.excludesfile is not define, Git will read
> the global exclude files in $XDG_CONFIG_HOME/git/ignore. Same goes for
> core.attributesfile in $XDG_CONFIG_HOME/git/attributes. If
> $XDG_CONFIG_HOME is either not set or empty, $HOME/.config will be
> used.
> 
> Signed-off-by: Huynh Khoi Nguyen NGUYEN <Huynh-Khoi-Nguyen.Nguyen@ensimag.imag.fr>
> Signed-off-by: Lucien KONG <Lucien.Kong@ensimag.imag.fr>
> Signed-off-by: Valentin DUPERRAY <Valentin.Duperray@ensimag.imag.fr>
> Signed-off-by: Thomas NGUY <Thomas.Nguy@ensimag.imag.fr>
> Signed-off-by: Franck JONAS <Franck.Jonas@ensimag.imag.fr>
> Signed-off-by: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
> ---
>  Documentation/git-config.txt    |   12 +++-
>  attr.c                          |   10 +++
>  builtin/config.c                |   28 ++++++---
>  cache.h                         |    1 +
>  config.c                        |   21 ++++---
>  dir.c                           |    4 +
>  path.c                          |   26 ++++++++
>  t/t1306-read-xdg-config-file.sh |  133 +++++++++++++++++++++++++++++++++++++++
>  8 files changed, 214 insertions(+), 21 deletions(-)
>  create mode 100755 t/t1306-read-xdg-config-file.sh
> 
[...]
Show 11 quoted lines
> diff --git a/attr.c b/attr.c
> index 303751f..441387f 100644
> --- a/attr.c
> +++ b/attr.c
> @@ -497,6 +497,9 @@ static int git_attr_system(void)
>  static void bootstrap_attr_stack(void)
>  {
>  	struct attr_stack *elem;
> +	char *xdg_attributes_file;
> +
> +	home_config_paths(NULL, &xdg_attributes_file, "attributes");
who free()'s xdg_attributes_file ?
[...]
Show 13 quoted lines
> diff --git a/config.c b/config.c
> index 71ef171..d1393b8 100644
> --- a/config.c
> +++ b/config.c
> @@ -929,7 +929,10 @@ int git_config_system(void)
>  int git_config_early(config_fn_t fn, void *data, const char *repo_config)
>  {
>  	int ret = 0, found = 0;
> -	const char *home = NULL;
> +	char *xdg_config = NULL;
> +	char *user_config = NULL;
> +
> +	home_config_paths(&user_config, &xdg_config, "config");
who free()'s user_config and xdg_config?
[...]
Show 13 quoted lines
> diff --git a/dir.c b/dir.c
> index ed1510f..e0c3589 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -1234,13 +1234,17 @@ int remove_dir_recursively(struct strbuf *path, int flag)
>  void setup_standard_excludes(struct dir_struct *dir)
>  {
>  	const char *path;
> +	char *xdg_path;
>  
>  	dir->exclude_per_dir = ".gitignore";
>  	path = git_path("info/exclude");
> +	home_config_paths(NULL, &xdg_path, "ignore");
ditto xdg_path ;-)
Show 43 quoted lines
>  	if (!access(path, R_OK))
>  		add_excludes_from_file(dir, path);
>  	if (excludes_file && !access(excludes_file, R_OK))
>  		add_excludes_from_file(dir, excludes_file);
> +	else if (!access(xdg_path, R_OK))
> +		add_excludes_from_file(dir, xdg_path);
>  }
>  
>  int remove_path(const char *name)
> diff --git a/path.c b/path.c
> index 6f2aa69..53f3f53 100644
> --- a/path.c
> +++ b/path.c
> @@ -122,6 +122,32 @@ char *git_path(const char *fmt, ...)
>  	return cleanup_path(pathname);
>  }
>  
> +void home_config_paths(char **global, char **xdg, char *file)
> +{
> +	char *xdg_home = getenv("XDG_CONFIG_HOME");
> +	char *home = getenv("HOME");
> +	char *to_free = NULL;
> +
> +	if (!home) {
> +		if (global)
> +			*global = NULL;
> +	} else {
> +		if (!xdg_home) {
> +			to_free = strdup(mkpath("%s/.config", home));
> +			xdg_home = to_free;
> +		}
> +		if (global)
> +			*global = xstrdup(mkpath("%s/.gitconfig", home));
> +	}
> +
> +	if (!xdg_home)
> +		*xdg = NULL;
> +	else
> +		*xdg = xstrdup(mkpath("%s/git/%s", xdg_home, file));
> +
> +	free(to_free);
> +}
> +

So, this re-introduces the bug addressed by commit 05bab3ea. The test number is now 29 (rather than 21) but the same test is failing; namely t3200-branch.sh test #29 (git branch -m q q2 without config should succeed).

In order to fix the bug, I created the patch given below (on top of this patch). (Note that it does not address the above issues).

HTH

ATB, Ramsay Jones

-- >8 --
From: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
Subject: [PATCH] path.c: Fix a static buffer overwrite bug by avoiding mkpath()

The v4 version of the "Read (but not write) from XDG configuration, XDG attributes and XDG ignore files" patch, re-introduced the bug addressed by commit 05bab3ea ("config.c: Fix a static buffer overwrite bug by avoiding mkpath()", 19-11-2011). Note that the patch refactored the code to determine the user (or home) configuration filename into a new function (home_config_paths()). In doing so, the new code once again uses mkpath() rather than mksnpath().

In order to fix the bug, we introduce a new variation of the mkpath() function, mkpathdup(), which avoids the use of the internal static buffers. As the name implies, the new function returns a pointer to the pathname as a dynamically allocated string. It is the callers responsibility to free the memory for the returned string.

Having introduced the new function, we can now replace the calls to 'xstrdup(mkpath(...))' in the home_config_paths() function with a call to mkpathdup() to achieve the same effect, without tickling the original bug.

(Also, note that the 'xstrdup(mkpath(...))' pattern occurs in several other places in the source ...)

Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---
 cache.h |    2 ++
 path.c  |   20 +++++++++++++++++---
 2 files changed, 19 insertions(+), 3 deletions(-)
diff --git a/cache.h b/cache.h
index 0632503..fbba2d6 100644
--- a/cache.h
+++ b/cache.h
@@ -619,6 +619,8 @@ extern char *git_snpath(char *buf, size_t n, const char *fmt, ...)
 	__attribute__((format (printf, 3, 4)));
 extern char *git_pathdup(const char *fmt, ...)
 	__attribute__((format (printf, 1, 2)));
+extern char *mkpathdup(const char *fmt, ...)
+	__attribute__((format (printf, 1, 2)));
 
 /* Return a statically allocated filename matching the sha1 signature */
 extern char *mkpath(const char *fmt, ...) __attribute__((format (printf, 1, 2)));
diff --git a/path.c b/path.c
index 53f3f53..ca29bdd 100644
--- a/path.c
+++ b/path.c
@@ -87,6 +87,20 @@ char *git_pathdup(const char *fmt, ...)
 	return xstrdup(path);
 }
 
+char *mkpathdup(const char *fmt, ...)
+{
+	char path[PATH_MAX];
+	va_list args;
+	unsigned len;
+
+	va_start(args, fmt);
+	len = vsnprintf(path, sizeof(path), fmt, args);
+	va_end(args);
+	if (len >= sizeof(path))
+		return xstrdup(bad_path);
+	return xstrdup(cleanup_path(path));
+}
+
 char *mkpath(const char *fmt, ...)
 {
 	va_list args;
@@ -133,17 +147,17 @@ void home_config_paths(char **global, char **xdg, char *file)
 			*global = NULL;
 	} else {
 		if (!xdg_home) {
-			to_free = strdup(mkpath("%s/.config", home));
+			to_free = mkpathdup("%s/.config", home);
 			xdg_home = to_free;
 		}
 		if (global)
-			*global = xstrdup(mkpath("%s/.gitconfig", home));
+			*global = mkpathdup("%s/.gitconfig", home);
 	}
 
 	if (!xdg_home)
 		*xdg = NULL;
 	else
-		*xdg = xstrdup(mkpath("%s/git/%s", xdg_home, file));
+		*xdg = mkpathdup("%s/git/%s", xdg_home, file);
 
 	free(to_free);
 }

-- 
1.7.10
Previous: Junio C HamanoNext: Junio C Hamano
Message 85 of 88 in “[PATCHv2] Possibility to read both from ~/.gitconfig and from $XDG_CONFIG_HOME/git/config”
  1. Huynh Khoi Nguyen NGUYENMay 30, 2012
  2. Junio C HamanoMay 30, 2012
  3. Ramsay JonesMay 31, 2012
  4. [PATCHv3] Read from XDG configuration file, not writeHuynh Khoi Nguyen NGUYEN, May 31, 2012
  5. Junio C HamanoMay 31, 2012
  6. [PATCHv4] Read (but not write) from XDG configuration, XDG attributes and XDG ignore filesHuynh Khoi Nguyen NGUYEN, Jun 1, 2012
  7. Matthieu MoyJun 2, 2012
  8. nguyenhu@minatec.inpg.frJun 2, 2012
  9. Matthieu MoyJun 2, 2012
  10. 1/4 Read (but not write) from $XDG_CONFIG_HOME/git/config fileHuynh Khoi Nguyen NGUYEN, Jun 3, 2012
  11. 2/4 Let core.excludesfile default to $XDG_CONFIG_HOME/git/ignoreHuynh Khoi Nguyen NGUYEN, Jun 3, 2012
  12. Matthieu MoyJun 4, 2012
  13. nguyenhu@minatec.inpg.frJun 5, 2012
  14. 3/4 Let core.attributesfile default to $XDG_CONFIG_HOME/git/attributesHuynh Khoi Nguyen NGUYEN, Jun 3, 2012
  15. 4/4 Write to $XDG_CONFIG_HOME/git/config fileHuynh Khoi Nguyen NGUYEN, Jun 3, 2012
  16. Matthieu MoyJun 4, 2012
  17. nguyenhu@minatec.inpg.frJun 5, 2012
  18. 1/4 Read (but not write) from $XDG_CONFIG_HOME/git/config fileHuynh Khoi Nguyen NGUYEN, Jun 6, 2012
  19. 2/4 Let core.excludesfile default to $XDG_CONFIG_HOME/git/ignoreHuynh Khoi Nguyen NGUYEN, Jun 6, 2012
  20. Junio C HamanoJun 7, 2012
  21. Matthieu MoyJun 8, 2012
  22. nguyenhu@minatec.inpg.frJun 8, 2012
  23. 3/4 Let core.attributesfile default to $XDG_CONFIG_HOME/git/attributesHuynh Khoi Nguyen NGUYEN, Jun 6, 2012
  24. 4/4 Write to $XDG_CONFIG_HOME/git/config fileHuynh Khoi Nguyen NGUYEN, Jun 6, 2012
  25. David AguilarJun 9, 2012
  26. Junio C HamanoJun 9, 2012
  27. David AguilarJun 9, 2012
  28. Matthieu MoyJun 10, 2012
  29. nguyenhu@minatec.inpg.frJun 11, 2012
  30. Junio C HamanoJun 7, 2012
  31. nguyenhu@minatec.inpg.frJun 8, 2012
  32. Ramsay JonesJun 12, 2012
  33. nguyenhu@minatec.inpg.frJun 8, 2012
  34. Erik Faye-LundJun 8, 2012
  35. nguyenhu@minatec.inpg.frJun 8, 2012
  36. Erik Faye-LundJun 8, 2012
  37. Junio C HamanoJun 8, 2012
  38. nguyenhu@minatec.inpg.frJun 9, 2012
  39. Junio C HamanoJun 10, 2012
  40. nguyenhu@minatec.inpg.frJun 10, 2012
  41. Erik Faye-LundJun 10, 2012
  42. nguyenhu@minatec.inpg.frJun 10, 2012
  43. Erik Faye-LundJun 10, 2012
  44. Junio C HamanoJun 11, 2012
  45. nguyenhu@minatec.inpg.frJun 11, 2012
  46. nguyenhu@minatec.inpg.frJun 11, 2012
  47. Erik Faye-LundJun 11, 2012
  48. 1/4 Read (but not write) from $XDG_CONFIG_HOME/git/config fileHuynh Khoi Nguyen Nguyen, Jun 12, 2012
  49. 2/4 Let core.excludesfile default to $XDG_CONFIG_HOME/git/ignoreHuynh Khoi Nguyen Nguyen, Jun 12, 2012
  50. 3/4 Let core.attributesfile default to $XDG_CONFIG_HOME/git/attributesHuynh Khoi Nguyen Nguyen, Jun 12, 2012
  51. 4/4 Write to $XDG_CONFIG_HOME/git/config fileHuynh Khoi Nguyen Nguyen, Jun 12, 2012
  52. Ramsay JonesJun 14, 2012
  53. Matthieu MoyJun 21, 2012
  54. Junio C HamanoJun 21, 2012
  55. 0/4 Git configuration directoryMatthieu Moy, Jun 22, 2012
  56. 1/4 config: read (but not write) from $XDG_CONFIG_HOME/git/config fileMatthieu Moy, Jun 22, 2012
  57. Thomas RastJul 12, 2012
  58. config: fix several access(NULL) callsMatthieu Moy, Jul 12, 2012
  59. Thomas RastJul 12, 2012
  60. Junio C HamanoJul 12, 2012
  61. Matthieu MoyJul 12, 2012
  62. Junio C HamanoJul 12, 2012
  63. Matthieu MoyJul 13, 2012
  64. config: fix several access(NULL) callsMatthieu Moy, Jul 13, 2012
  65. Jeff KingJul 13, 2012
  66. Matthieu MoyJul 13, 2012
  67. Thomas RastJul 13, 2012
  68. Matthieu MoyJul 13, 2012
  69. Junio C HamanoJul 13, 2012
  70. Matthieu MoyJul 16, 2012
  71. Junio C HamanoJul 16, 2012
  72. Matthieu MoyJul 16, 2012
  73. Junio C HamanoJul 16, 2012
  74. 2/4 Let core.excludesfile default to $XDG_CONFIG_HOME/git/ignoreMatthieu Moy, Jun 22, 2012
  75. 3/4 Let core.attributesfile default to $XDG_CONFIG_HOME/git/ignoreMatthieu Moy, Jun 22, 2012
  76. Junio C HamanoJun 22, 2012
  77. Matthieu MoyJun 25, 2012
  78. Junio C HamanoJun 25, 2012
  79. Matthieu MoyJun 25, 2012
  80. 4/4 config: write to $XDG_CONFIG_HOME/git/config file if appropriateMatthieu Moy, Jun 22, 2012
  81. Junio C HamanoJun 22, 2012
  82. Matthieu MoyJun 25, 2012
  83. Junio C HamanoJun 25, 2012
  84. Junio C HamanoJun 22, 2012
  85. Ramsay JonesJun 4, 2012
  86. Junio C HamanoJun 4, 2012
  87. Ramsay JonesJun 12, 2012
  88. nguyenhu@minatec.inpg.frJun 5, 2012

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.