Re: [REGRESSION] git-wrapper to run-commands codepath regression

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

 



On Mon, Apr 18, 2011 at 05:11:02PM -0400, Jeff King wrote:

> The good news is that the bug is trivial. It bisects Jonathan's ebec842
> (run-command: prettify -D_FORTIFY_SOURCE workaround, 2011-03-16), which
> introduces:
> 
> -       unused = write(child_err, "fatal: ", 7);
> -       unused = write(child_err, msg, len);
> -       unused = write(child_err, "\n", 1);
> +       if (write(child_err, "fatal: ", 7) ||
> +           write(child_err, msg, len) ||
> +           write(child_err, "\n", 1))
> +               ; /* yes, gcc -D_FORTIFY_SOURCE, we know there was an error. */
> 
> Stare at that for a minute and see if you can guess what's wrong. :)

And here's the fix.

-- >8 --
Subject: [PATCH] run-command: fix broken error messages from child

After we fork, we try to exec the child; if exec fails, we
write an error message and exit. We ignore the return value
of write, since there's nothing we can do about it.

Commit ebec842 turned this into a conditional to make
-D_FORTIFY_SOURCE happy with the ignored return value, but
botched the change so that we never write more than
"fatal:".

Write will return the number of bytes written, so the
conditional as written will always appear as an error. Of
course we don't actually do anything for the error, but
the short-circuit logic means we never execute the
subsequent write()s, giving us a truncated error message.

Signed-off-by: Jeff King <peff@xxxxxxxx>
---
 run-command.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/run-command.c b/run-command.c
index 8619c76..508a4c6 100644
--- a/run-command.c
+++ b/run-command.c
@@ -83,9 +83,9 @@ static NORETURN void die_child(const char *err, va_list params)
 	if (len > sizeof(msg))
 		len = sizeof(msg);
 
-	if (write(child_err, "fatal: ", 7) ||
-	    write(child_err, msg, len) ||
-	    write(child_err, "\n", 1))
+	if (write(child_err, "fatal: ", 7) < 0 ||
+	    write(child_err, msg, len) < 0 ||
+	    write(child_err, "\n", 1) < 0)
 		; /* yes, gcc -D_FORTIFY_SOURCE, we know there was an error. */
 	exit(128);
 }
-- 
1.7.5.rc2.3.g728b2

--
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]