From: Erik Faye-Lund <kusmabite@gmail.com>
Date: Wed, 16 Mar 2011 15:58:03 GMT
Subject: Re: [PATCH] system_path: use a static buffer
Message-ID: <AANLkTinEMzezKdfGUrwKv7sJ+tSK5duYM6XZMvBa-yj3@mail.gmail.com>
In-Reply-To: <1300274770-4798-1-git-send-email-cmn@elego.de>

On Wed, Mar 16, 2011 at 12:26 PM, Carlos Martín Nieto <cmn@elego.de> wrote:
> Make system_path behave like the other path functions by using a
> static buffer, fixing a memory leak.
>
> Also make sure the prefix pointer is always initialized to either
> PREFIX or NULL.
>
> Signed-off-by: Carlos Martín Nieto <cmn@elego.de>
> ---
>
> This fixes the leak I was trying to fix with my original patch, but
> this seems much cleaner
>
>  exec_cmd.c |    9 ++++-----
>  1 files changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/exec_cmd.c b/exec_cmd.c
> index 38545e8..12ce017 100644
> --- a/exec_cmd.c
> +++ b/exec_cmd.c
> @@ -9,11 +9,11 @@ static const char *argv0_path;
>  const char *system_path(const char *path)
>  {
>  #ifdef RUNTIME_PREFIX
> -       static const char *prefix;
> +       static const char *prefix = NULL;
>  #else
>        static const char *prefix = PREFIX;
>  #endif
> -       struct strbuf d = STRBUF_INIT;
> +       static char buf[PATH_MAX+1];

Why PATH_MAX + 1? POSIX says that PATH_MAX includes the null-termination...

> @@ -33,9 +33,8 @@ const char *system_path(const char *path)
>        }
>  #endif
>
> -       strbuf_addf(&d, "%s/%s", prefix, path);
> -       path = strbuf_detach(&d, NULL);
> -       return path;
> +       snprintf(buf, PATH_MAX, "%s/%s", prefix, path);

Perhaps "snprintf(buf, sizeof(buf) - 1, "%s/%s", prefix, path);" instead?