Re: [PATCH v2 06/10] setup_git_env: use git_pathdup instead of xmalloc + sprintf

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Jun 25, 2014 at 3:58 AM, Jeff King <peff@xxxxxxxx> wrote:
> Here's a replacement patch that handles this (and just drops the ugly
> mallocs as a side effect).

Shortly after I wrote my email, I thought about getenvdup() and look
for similar getenv/xstrdup patterns. But I saw only one in config.c.
So let's forget about it. Your patch looks good.

>
> -- >8 --
> Subject: [PATCH] setup_git_env: copy getenv return value
>
> The return value of getenv is not guaranteed to survive
> across further invocations of setenv or even getenv. When we
> are assigning it to globals that last the lifetime of the
> program, we should make our own copy.
>
> Signed-off-by: Jeff King <peff@xxxxxxxx>
> ---
>  environment.c | 22 +++++++++-------------
>  1 file changed, 9 insertions(+), 13 deletions(-)
>
> diff --git a/environment.c b/environment.c
> index 4dac5e9..efb2fa0 100644
> --- a/environment.c
> +++ b/environment.c
> @@ -124,6 +124,12 @@ static char *expand_namespace(const char *raw_namespace)
>         return strbuf_detach(&buf, NULL);
>  }
>
> +static char *git_path_from_env(const char *envvar, const char *path)
> +{
> +       const char *value = getenv(envvar);
> +       return value ? xstrdup(value) : git_pathdup(path);
> +}
> +
>  static void setup_git_env(void)
>  {
>         const char *gitfile;
> @@ -134,19 +140,9 @@ static void setup_git_env(void)
>                 git_dir = DEFAULT_GIT_DIR_ENVIRONMENT;
>         gitfile = read_gitfile(git_dir);
>         git_dir = xstrdup(gitfile ? gitfile : git_dir);
> -       git_object_dir = getenv(DB_ENVIRONMENT);
> -       if (!git_object_dir) {
> -               git_object_dir = xmalloc(strlen(git_dir) + 9);
> -               sprintf(git_object_dir, "%s/objects", git_dir);
> -       }
> -       git_index_file = getenv(INDEX_ENVIRONMENT);
> -       if (!git_index_file) {
> -               git_index_file = xmalloc(strlen(git_dir) + 7);
> -               sprintf(git_index_file, "%s/index", git_dir);
> -       }
> -       git_graft_file = getenv(GRAFT_ENVIRONMENT);
> -       if (!git_graft_file)
> -               git_graft_file = git_pathdup("info/grafts");
> +       git_object_dir = git_path_from_env(DB_ENVIRONMENT, "objects");
> +       git_index_file = git_path_from_env(INDEX_ENVIRONMENT, "index");
> +       git_graft_file = git_path_from_env(GRAFT_ENVIRONMENT, "info/grafts");
>         if (getenv(NO_REPLACE_OBJECTS_ENVIRONMENT))
>                 check_replace_refs = 0;
>         namespace = expand_namespace(getenv(GIT_NAMESPACE_ENVIRONMENT));
> --
> 2.0.0.566.gfe3e6b2
>



-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]