[PATCH] run-command: avoid undefined behavior in exists_in_PATH

[PATCH] run-command: avoid undefined behavior in exists_in_PATH

[brian m. carlson]

In this function, we free the pointer we get from locate_in_PATH and
then check whether it's NULL.  However, this is undefined behavior if
the pointer is non-NULL, since the C standard no longer permits us to
use a valid pointer after freeing it.

The only case in which the C standard would permit this to be defined
behavior is if r were NULL, since it states that in such a case "no
action occurs" as a result of calling free.

It's easy to suggest that this is not likely to be a problem, but we
know that GCC does aggressively exploit the fact that undefined
behavior can never occur to optimize and rewrite code, even when that's
contrary to the expectations of the programmer.  It is, in fact, very
common for it to omit NULL pointer checks, just as we have here.

Since it's easy to fix, let's do so, and avoid a potential headache in
the future.

Noticed-by: Miriam R. <mirucam@gmail.com>
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 run-command.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/run-command.c b/run-command.c
index 9942f120a9..f5e1149f9b 100644
--- a/run-command.c
+++ b/run-command.c
@@ -213,8 +213,9 @@ static char *locate_in_PATH(const char *file)
 static int exists_in_PATH(const char *file)
 {
 	char *r = locate_in_PATH(file);
+	int found = r != NULL;
 	free(r);
-	return r != NULL;
+	return found;
 }
 
 int sane_execvp(const char *file, char * const argv[])

Re: [PATCH] run-command: avoid undefined behavior in exists_in_PATH

[Jonathan Nieder]

brian m. carlson wrote:

> In this function, we free the pointer we get from locate_in_PATH and
> then check whether it's NULL.  However, this is undefined behavior if
> the pointer is non-NULL, since the C standard no longer permits us to
> use a valid pointer after freeing it.
[...]
> Noticed-by: Miriam R. <mirucam@gmail.com>
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
>  run-command.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

This API that forces the caller to deal with the allocated result when
they never asked for it seems a bit inconvenient.  Should we clean it up
a little?  Something like this (on top):

-- >8 --
Subject: run-command: let caller pass in buffer to locate_in_PATH

Instead of returning a buffer that the caller is responsible for
freeing, use a strbuf output parameter to record the path to the
searched-for program.

This makes ownership a little easier to reason about, since the owning
code declares the buffer.  It's a good habit to follow because it
allows buffer reuse when calling such a function in a loop.

It also allows the caller exists_in_PATH that does not care about the
path to the command to be slightly simplified, by allowing a NULL
output parameter that means that locate_in_PATH should take care of
allocating and freeing its temporary buffer.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 run-command.c | 51 +++++++++++++++++++++++++++++----------------------
 1 file changed, 29 insertions(+), 22 deletions(-)

diff --git i/run-command.c w/run-command.c
index f5e1149f9b..a6dc38396a 100644
--- i/run-command.c
+++ w/run-command.c
@@ -170,52 +170,57 @@ int is_executable(const char *name)
  * The caller should ensure that file contains no directory
  * separators.
  *
- * Returns the path to the command, as found in $PATH or NULL if the
- * command could not be found.  The caller inherits ownership of the memory
- * used to store the resultant path.
+ * Returns a boolean indicating whether the command was found in $PATH.
+ * The path to the command is recorded in the strbuf 'out', if supplied.
  *
  * This should not be used on Windows, where the $PATH search rules
  * are more complicated (e.g., a search for "foo" should find
  * "foo.exe").
  */
-static char *locate_in_PATH(const char *file)
+static int locate_in_PATH(const char *file, struct strbuf *out)
 {
 	const char *p = getenv("PATH");
 	struct strbuf buf = STRBUF_INIT;
 
-	if (!p || !*p)
-		return NULL;
+	if (!out)
+		out = &buf;
+
+	if (!p || !*p) {
+		strbuf_reset(out);
+		strbuf_release(&buf);
+		return 0;
+	}
 
 	while (1) {
 		const char *end = strchrnul(p, ':');
 
-		strbuf_reset(&buf);
+		strbuf_reset(out);
 
 		/* POSIX specifies an empty entry as the current directory. */
 		if (end != p) {
-			strbuf_add(&buf, p, end - p);
-			strbuf_addch(&buf, '/');
+			strbuf_add(out, p, end - p);
+			strbuf_addch(out, '/');
 		}
-		strbuf_addstr(&buf, file);
+		strbuf_addstr(out, file);
 
-		if (is_executable(buf.buf))
-			return strbuf_detach(&buf, NULL);
+		if (is_executable(out->buf)) {
+			strbuf_release(&buf);
+			return 1;
+		}
 
 		if (!*end)
 			break;
 		p = end + 1;
 	}
 
+	strbuf_reset(out);
 	strbuf_release(&buf);
-	return NULL;
+	return 0;
 }
 
 static int exists_in_PATH(const char *file)
 {
-	char *r = locate_in_PATH(file);
-	int found = r != NULL;
-	free(r);
-	return found;
+	return locate_in_PATH(file, NULL);
 }
 
 int sane_execvp(const char *file, char * const argv[])
@@ -427,15 +432,17 @@ static int prepare_cmd(struct argv_array *out, const struct child_process *cmd)
 	 * directly.
 	 */
 	if (!strchr(out->argv[1], '/')) {
-		char *program = locate_in_PATH(out->argv[1]);
-		if (program) {
-			free((char *)out->argv[1]);
-			out->argv[1] = program;
-		} else {
+		struct strbuf program = STRBUF_INIT;
+
+		if (!locate_in_PATH(out->argv[1], &program)) {
 			argv_array_clear(out);
+			strbuf_release(&program);
 			errno = ENOENT;
 			return -1;
 		}
+
+		free((char *)out->argv[1]);
+		out->argv[1] = strbuf_detach(&program, NULL);
 	}
 
 	return 0;

Re: [PATCH] run-command: avoid undefined behavior in exists_in_PATH

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 1905 bytes --]

On 2020-01-07 at 02:04:25, Jonathan Nieder wrote:
> brian m. carlson wrote:
> 
> > In this function, we free the pointer we get from locate_in_PATH and
> > then check whether it's NULL.  However, this is undefined behavior if
> > the pointer is non-NULL, since the C standard no longer permits us to
> > use a valid pointer after freeing it.
> [...]
> > Noticed-by: Miriam R. <mirucam@gmail.com>
> > Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> > ---
> >  run-command.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
> 
> This API that forces the caller to deal with the allocated result when
> they never asked for it seems a bit inconvenient.  Should we clean it up
> a little?  Something like this (on top):
> 
> -- >8 --
> Subject: run-command: let caller pass in buffer to locate_in_PATH
> 
> Instead of returning a buffer that the caller is responsible for
> freeing, use a strbuf output parameter to record the path to the
> searched-for program.
> 
> This makes ownership a little easier to reason about, since the owning
> code declares the buffer.  It's a good habit to follow because it
> allows buffer reuse when calling such a function in a loop.
> 
> It also allows the caller exists_in_PATH that does not care about the
> path to the command to be slightly simplified, by allowing a NULL
> output parameter that means that locate_in_PATH should take care of
> allocating and freeing its temporary buffer.

Sure, I think this is a nice improvement.  The user can reuse the buffer
in a loop if they want by resetting it, which as you point out would be
convenient (and potentially more efficient).  And we're already using
one internally, so there's no reason not to just pass one in.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]

Re: [PATCH] run-command: avoid undefined behavior in exists_in_PATH

[Bryan Turner]

On Mon, Jan 6, 2020 at 6:04 PM Jonathan Nieder <jrnieder@gmail.com> wrote:
>
> brian m. carlson wrote:
>
> > In this function, we free the pointer we get from locate_in_PATH and
> > then check whether it's NULL.  However, this is undefined behavior if
> > the pointer is non-NULL, since the C standard no longer permits us to
> > use a valid pointer after freeing it.
> [...]
> > Noticed-by: Miriam R. <mirucam@gmail.com>
> > Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> > ---
> >  run-command.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
>
> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
>
> This API that forces the caller to deal with the allocated result when
> they never asked for it seems a bit inconvenient.  Should we clean it up
> a little?  Something like this (on top):
>
> -- >8 --
> Subject: run-command: let caller pass in buffer to locate_in_PATH
>
> Instead of returning a buffer that the caller is responsible for
> freeing, use a strbuf output parameter to record the path to the
> searched-for program.
>
> This makes ownership a little easier to reason about, since the owning
> code declares the buffer.  It's a good habit to follow because it
> allows buffer reuse when calling such a function in a loop.
>
> It also allows the caller exists_in_PATH that does not care about the
> path to the command to be slightly simplified, by allowing a NULL
> output parameter that means that locate_in_PATH should take care of
> allocating and freeing its temporary buffer.
>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> ---
>  run-command.c | 51 +++++++++++++++++++++++++++++----------------------
>  1 file changed, 29 insertions(+), 22 deletions(-)
>
> diff --git i/run-command.c w/run-command.c
> index f5e1149f9b..a6dc38396a 100644
> --- i/run-command.c
> +++ w/run-command.c
> @@ -170,52 +170,57 @@ int is_executable(const char *name)
>   * The caller should ensure that file contains no directory
>   * separators.
>   *
> - * Returns the path to the command, as found in $PATH or NULL if the
> - * command could not be found.  The caller inherits ownership of the memory
> - * used to store the resultant path.
> + * Returns a boolean indicating whether the command was found in $PATH.
> + * The path to the command is recorded in the strbuf 'out', if supplied.
>   *
>   * This should not be used on Windows, where the $PATH search rules
>   * are more complicated (e.g., a search for "foo" should find
>   * "foo.exe").
>   */
> -static char *locate_in_PATH(const char *file)
> +static int locate_in_PATH(const char *file, struct strbuf *out)
>  {
>         const char *p = getenv("PATH");
>         struct strbuf buf = STRBUF_INIT;
>
> -       if (!p || !*p)
> -               return NULL;
> +       if (!out)
> +               out = &buf;
> +
> +       if (!p || !*p) {
> +               strbuf_reset(out);

Since the while loop and this block both call strbuf_reset(out);, is
there a reason not to do:
if (out)
        strbuf_reset(out);
} else {
        out = &buf;
}
above? The loop below would still need its own reset, but it could do
that when it decides to loop.

> +               strbuf_release(&buf);
> +               return 0;
> +       }
>
>         while (1) {
>                 const char *end = strchrnul(p, ':');
>
> -               strbuf_reset(&buf);
> +               strbuf_reset(out);

This reset would be removed

>
>                 /* POSIX specifies an empty entry as the current directory. */
>                 if (end != p) {
> -                       strbuf_add(&buf, p, end - p);
> -                       strbuf_addch(&buf, '/');
> +                       strbuf_add(out, p, end - p);
> +                       strbuf_addch(out, '/');
>                 }
> -               strbuf_addstr(&buf, file);
> +               strbuf_addstr(out, file);
>
> -               if (is_executable(buf.buf))
> -                       return strbuf_detach(&buf, NULL);
> +               if (is_executable(out->buf)) {
> +                       strbuf_release(&buf);
> +                       return 1;
> +               }
>

We'd call strbuf_reset(out); here instead, before we break or loop.

>                 if (!*end)
>                         break;
>                 p = end + 1
>         }
>
> +       strbuf_reset(out);

And we could leave this one out; if we make it here, we'd know the
buffer was already reset.

>         strbuf_release(&buf);
> -       return NULL;
> +       return 0;
>  }
>
>  static int exists_in_PATH(const char *file)
>  {
> -       char *r = locate_in_PATH(file);
> -       int found = r != NULL;
> -       free(r);
> -       return found;
> +       return locate_in_PATH(file, NULL);
>  }
>
>  int sane_execvp(const char *file, char * const argv[])
> @@ -427,15 +432,17 @@ static int prepare_cmd(struct argv_array *out, const struct child_process *cmd)
>          * directly.
>          */
>         if (!strchr(out->argv[1], '/')) {
> -               char *program = locate_in_PATH(out->argv[1]);
> -               if (program) {
> -                       free((char *)out->argv[1]);
> -                       out->argv[1] = program;
> -               } else {
> +               struct strbuf program = STRBUF_INIT;
> +
> +               if (!locate_in_PATH(out->argv[1], &program)) {
>                         argv_array_clear(out);
> +                       strbuf_release(&program);
>                         errno = ENOENT;
>                         return -1;
>                 }
> +
> +               free((char *)out->argv[1]);
> +               out->argv[1] = strbuf_detach(&program, NULL);
>         }
>
>         return 0;

Just a thought. (Pardon the noise from the peanut gallery!)

Best regards,
Bryan Turner

Re: [PATCH] run-command: avoid undefined behavior in exists_in_PATH

[Bryan Turner]

On Mon, Jan 6, 2020 at 7:40 PM Bryan Turner <bturner@atlassian.com> wrote:
>
> On Mon, Jan 6, 2020 at 6:04 PM Jonathan Nieder <jrnieder@gmail.com> wrote:
> >
> > brian m. carlson wrote:
> >
> > > In this function, we free the pointer we get from locate_in_PATH and
> > > then check whether it's NULL.  However, this is undefined behavior if
> > > the pointer is non-NULL, since the C standard no longer permits us to
> > > use a valid pointer after freeing it.
> > [...]
> > > Noticed-by: Miriam R. <mirucam@gmail.com>
> > > Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> > > ---
> > >  run-command.c | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
> >
> > This API that forces the caller to deal with the allocated result when
> > they never asked for it seems a bit inconvenient.  Should we clean it up
> > a little?  Something like this (on top):
> >
> > -- >8 --
> > Subject: run-command: let caller pass in buffer to locate_in_PATH
> >
> > Instead of returning a buffer that the caller is responsible for
> > freeing, use a strbuf output parameter to record the path to the
> > searched-for program.
> >
> > This makes ownership a little easier to reason about, since the owning
> > code declares the buffer.  It's a good habit to follow because it
> > allows buffer reuse when calling such a function in a loop.
> >
> > It also allows the caller exists_in_PATH that does not care about the
> > path to the command to be slightly simplified, by allowing a NULL
> > output parameter that means that locate_in_PATH should take care of
> > allocating and freeing its temporary buffer.
> >
> > Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> > ---
> >  run-command.c | 51 +++++++++++++++++++++++++++++----------------------
> >  1 file changed, 29 insertions(+), 22 deletions(-)
> >
> > diff --git i/run-command.c w/run-command.c
> > index f5e1149f9b..a6dc38396a 100644
> > --- i/run-command.c
> > +++ w/run-command.c
> > @@ -170,52 +170,57 @@ int is_executable(const char *name)
> >   * The caller should ensure that file contains no directory
> >   * separators.
> >   *
> > - * Returns the path to the command, as found in $PATH or NULL if the
> > - * command could not be found.  The caller inherits ownership of the memory
> > - * used to store the resultant path.
> > + * Returns a boolean indicating whether the command was found in $PATH.
> > + * The path to the command is recorded in the strbuf 'out', if supplied.
> >   *
> >   * This should not be used on Windows, where the $PATH search rules
> >   * are more complicated (e.g., a search for "foo" should find
> >   * "foo.exe").
> >   */
> > -static char *locate_in_PATH(const char *file)
> > +static int locate_in_PATH(const char *file, struct strbuf *out)
> >  {
> >         const char *p = getenv("PATH");
> >         struct strbuf buf = STRBUF_INIT;
> >
> > -       if (!p || !*p)
> > -               return NULL;
> > +       if (!out)
> > +               out = &buf;
> > +
> > +       if (!p || !*p) {
> > +               strbuf_reset(out);
>
> Since the while loop and this block both call strbuf_reset(out);, is
> there a reason not to do:
> if (out)
>         strbuf_reset(out);
> } else {
>         out = &buf;
> }
> above? The loop below would still need its own reset, but it could do
> that when it decides to loop.

Ignore my incorrect braces; force of habit. I meant:
if (out)
        strbuf_reset(out);
else
        out = &buf;

>
> > +               strbuf_release(&buf);
> > +               return 0;
> > +       }
> >
> >         while (1) {
> >                 const char *end = strchrnul(p, ':');
> >
> > -               strbuf_reset(&buf);
> > +               strbuf_reset(out);
>
> This reset would be removed
>
> >
> >                 /* POSIX specifies an empty entry as the current directory. */
> >                 if (end != p) {
> > -                       strbuf_add(&buf, p, end - p);
> > -                       strbuf_addch(&buf, '/');
> > +                       strbuf_add(out, p, end - p);
> > +                       strbuf_addch(out, '/');
> >                 }
> > -               strbuf_addstr(&buf, file);
> > +               strbuf_addstr(out, file);
> >
> > -               if (is_executable(buf.buf))
> > -                       return strbuf_detach(&buf, NULL);
> > +               if (is_executable(out->buf)) {
> > +                       strbuf_release(&buf);
> > +                       return 1;
> > +               }
> >
>
> We'd call strbuf_reset(out); here instead, before we break or loop.
>
> >                 if (!*end)
> >                         break;
> >                 p = end + 1
> >         }
> >
> > +       strbuf_reset(out);
>
> And we could leave this one out; if we make it here, we'd know the
> buffer was already reset.
>
> >         strbuf_release(&buf);
> > -       return NULL;
> > +       return 0;
> >  }
> >
> >  static int exists_in_PATH(const char *file)
> >  {
> > -       char *r = locate_in_PATH(file);
> > -       int found = r != NULL;
> > -       free(r);
> > -       return found;
> > +       return locate_in_PATH(file, NULL);
> >  }
> >
> >  int sane_execvp(const char *file, char * const argv[])
> > @@ -427,15 +432,17 @@ static int prepare_cmd(struct argv_array *out, const struct child_process *cmd)
> >          * directly.
> >          */
> >         if (!strchr(out->argv[1], '/')) {
> > -               char *program = locate_in_PATH(out->argv[1]);
> > -               if (program) {
> > -                       free((char *)out->argv[1]);
> > -                       out->argv[1] = program;
> > -               } else {
> > +               struct strbuf program = STRBUF_INIT;
> > +
> > +               if (!locate_in_PATH(out->argv[1], &program)) {
> >                         argv_array_clear(out);
> > +                       strbuf_release(&program);
> >                         errno = ENOENT;
> >                         return -1;
> >                 }
> > +
> > +               free((char *)out->argv[1]);
> > +               out->argv[1] = strbuf_detach(&program, NULL);
> >         }
> >
> >         return 0;
>
> Just a thought. (Pardon the noise from the peanut gallery!)
>
> Best regards,
> Bryan Turner

Re: [PATCH] run-command: avoid undefined behavior in exists_in_PATH

[Jeff King]

On Tue, Jan 07, 2020 at 01:36:40AM +0000, brian m. carlson wrote:

> In this function, we free the pointer we get from locate_in_PATH and
> then check whether it's NULL.  However, this is undefined behavior if
> the pointer is non-NULL, since the C standard no longer permits us to
> use a valid pointer after freeing it.
> 
> The only case in which the C standard would permit this to be defined
> behavior is if r were NULL, since it states that in such a case "no
> action occurs" as a result of calling free.
> 
> It's easy to suggest that this is not likely to be a problem, but we
> know that GCC does aggressively exploit the fact that undefined
> behavior can never occur to optimize and rewrite code, even when that's
> contrary to the expectations of the programmer.  It is, in fact, very
> common for it to omit NULL pointer checks, just as we have here.

OK, I agree it makes sense to be on the safe side here (and the patch is
obviously the right fix).

> Noticed-by: Miriam R. <mirucam@gmail.com>
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>

I think Miriam actually posted the same patch in her initial email:

  https://lore.kernel.org/git/CAN7CjDDBA0ZoCG9aaQf5rg3gxqny=EjR6v6jE1mnxvUJQSF_0Q@mail.gmail.com/

I don't know how we want to handle authorship.

-Peff

Re: [PATCH] run-command: avoid undefined behavior in exists_in_PATH

[Jeff King]

On Mon, Jan 06, 2020 at 06:04:25PM -0800, Jonathan Nieder wrote:

> -- >8 --
> Subject: run-command: let caller pass in buffer to locate_in_PATH
> 
> Instead of returning a buffer that the caller is responsible for
> freeing, use a strbuf output parameter to record the path to the
> searched-for program.
> 
> This makes ownership a little easier to reason about, since the owning
> code declares the buffer.  It's a good habit to follow because it
> allows buffer reuse when calling such a function in a loop.
> 
> It also allows the caller exists_in_PATH that does not care about the
> path to the command to be slightly simplified, by allowing a NULL
> output parameter that means that locate_in_PATH should take care of
> allocating and freeing its temporary buffer.
>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> ---
>  run-command.c | 51 +++++++++++++++++++++++++++++----------------------
>  1 file changed, 29 insertions(+), 22 deletions(-)

I dunno. Now the rules inside locate_in_PATH() are more complicated, and
we have an unusual boolean return from the function.

I admit I don't overly care either way, as there are literally two
callers (and not likely to be more). So I'd probably just leave it
alone, but I'm not opposed to the patch if people think it's a good
idea.

-Peff

Re: [PATCH] run-command: avoid undefined behavior in exists_in_PATH

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Tue, Jan 07, 2020 at 01:36:40AM +0000, brian m. carlson wrote:
>
>> In this function, we free the pointer we get from locate_in_PATH and
>> then check whether it's NULL.  However, this is undefined behavior if
>> the pointer is non-NULL, since the C standard no longer permits us to
>> use a valid pointer after freeing it.
>> 
>> The only case in which the C standard would permit this to be defined
>> behavior is if r were NULL, since it states that in such a case "no
>> action occurs" as a result of calling free.
>> 
>> It's easy to suggest that this is not likely to be a problem, but we
>> know that GCC does aggressively exploit the fact that undefined
>> behavior can never occur to optimize and rewrite code, even when that's
>> contrary to the expectations of the programmer.  It is, in fact, very
>> common for it to omit NULL pointer checks, just as we have here.
>
> OK, I agree it makes sense to be on the safe side here (and the patch is
> obviously the right fix).
>
>> Noticed-by: Miriam R. <mirucam@gmail.com>
>> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
>
> I think Miriam actually posted the same patch in her initial email:
>
>   https://lore.kernel.org/git/CAN7CjDDBA0ZoCG9aaQf5rg3gxqny=EjR6v6jE1mnxvUJQSF_0Q@mail.gmail.com/
>
> I don't know how we want to handle authorship.

I think the explanation in the log message has as much value as, if
not more than, the actual patch text, in this case.  Noticed-by: may
be striking the right balance.

Thanks, all.

Re: [PATCH] run-command: avoid undefined behavior in exists_in_PATH

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 638 bytes --]

On 2020-01-07 at 11:01:45, Jeff King wrote:
> > Noticed-by: Miriam R. <mirucam@gmail.com>
> > Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> 
> I think Miriam actually posted the same patch in her initial email:
> 
>   https://lore.kernel.org/git/CAN7CjDDBA0ZoCG9aaQf5rg3gxqny=EjR6v6jE1mnxvUJQSF_0Q@mail.gmail.com/
> 
> I don't know how we want to handle authorship.

I don't feel strongly about holding authorship; I'm happy to have her
name on it instead of mine since she did propose a solution.  I just
care that we fix it.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]

Re: [PATCH] run-command: avoid undefined behavior in exists_in_PATH

[Miriam R.]

El mié., 8 ene. 2020 a las 3:47, brian m. carlson
(<sandals@crustytoothpaste.net>) escribió:
>
> On 2020-01-07 at 11:01:45, Jeff King wrote:
> > > Noticed-by: Miriam R. <mirucam@gmail.com>
> > > Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> >
> > I think Miriam actually posted the same patch in her initial email:
> >
> >   https://lore.kernel.org/git/CAN7CjDDBA0ZoCG9aaQf5rg3gxqny=EjR6v6jE1mnxvUJQSF_0Q@mail.gmail.com/
> >
> > I don't know how we want to handle authorship.
>
> I don't feel strongly about holding authorship; I'm happy to have her
> name on it instead of mine since she did propose a solution.  I just
> care that we fix it.
> --
Hi,
my mentor (Christian Couder <chriscool@tuxfamily.org>) was who
detected the problem and sent me the solution, I only asked the
community. I think his name should be in the patch instead of mine.

Best,
Miriam
> brian m. carlson: Houston, Texas, US
> OpenPGP: https://keybase.io/bk2204

Re: [PATCH] run-command: avoid undefined behavior in exists_in_PATH

[Christian Couder]

On Wed, Jan 8, 2020 at 10:16 AM Miriam R. <mirucam@gmail.com> wrote:
>
> El mié., 8 ene. 2020 a las 3:47, brian m. carlson
> (<sandals@crustytoothpaste.net>) escribió:

> > > I think Miriam actually posted the same patch in her initial email:
> > >
> > >   https://lore.kernel.org/git/CAN7CjDDBA0ZoCG9aaQf5rg3gxqny=EjR6v6jE1mnxvUJQSF_0Q@mail.gmail.com/
> > >
> > > I don't know how we want to handle authorship.
> >
> > I don't feel strongly about holding authorship; I'm happy to have her
> > name on it instead of mine since she did propose a solution.  I just
> > care that we fix it.

> my mentor (Christian Couder <chriscool@tuxfamily.org>) was who
> detected the problem and sent me the solution, I only asked the
> community. I think his name should be in the patch instead of mine.

I am ok with not having my name anywhere and leaving the patch as is.