From: Jonathan Nieder <jrnieder@gmail.com>
To: git@vger.kernel.org
Cc: Michael Wookey <michaelwookey@gmail.com>,
Markus Heidelberg <markus.heidelberg@web.de>,
Junio C Hamano <gitster@pobox.com>
Subject: [PATCH] run-command: prettify -D_FORTIFY_SOURCE workaround
Date: Tue, 15 Mar 2011 22:51:52 -0500 [thread overview]
Message-ID: <20110316035135.GA30348@elie> (raw)
In-Reply-To: <d2e97e801001291438k21a652cakb05ec34fc8bee227@mail.gmail.com>
Current gcc + glibc with -D_FORTIFY_SOURCE try very aggressively to
protect against a programming style which uses write(...) without
checking the return value for errors. Even the usual hint of casting
to (void) does not suppress the warning.
Sometimes when there is an output error, especially right before exit,
there really is nothing to be done. The obvious solution, adopted in
v1.7.0.3~20^2 (run-command.c: fix build warnings on Ubuntu,
2010-01-30), is to save the return value to a dummy variable:
ssize_t dummy;
dummy = write(...);
But that (1) is ugly and (2) triggers -Wunused-but-set-variable
warnings with gcc-4.6 -Wall, so we are not much better off than when
we started.
Instead, use an "if" statement with an empty body to make the intent
clear.
if (write(...))
; /* yes, yes, there was an error. */
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Hi,
Michael Wookey wrote:
> Although this will fix the build warnings, I am unsure if there is a
> better way to achieve the same result. Using "(void)write(...)" still
> gives warnings and I am unaware of any annotations that will silence
> gcc.
It's been a long time (and meanwhile the patch has been working;
thanks!). How about something like this?
run-command.c | 12 ++++++------
1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/run-command.c b/run-command.c
index 3206d61..5b68907 100644
--- a/run-command.c
+++ b/run-command.c
@@ -67,21 +67,21 @@ static int child_notifier = -1;
static void notify_parent(void)
{
- ssize_t unused;
- unused = write(child_notifier, "", 1);
+ if (write(child_notifier, "", 1))
+ ; /* ok. */
}
static NORETURN void die_child(const char *err, va_list params)
{
char msg[4096];
- ssize_t unused;
int len = vsnprintf(msg, sizeof(msg), err, params);
if (len > sizeof(msg))
len = sizeof(msg);
- 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))
+ ; /* ok. */
exit(128);
}
--
1.7.4.1
next prev parent reply other threads:[~2011-03-16 3:52 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-01-29 22:38 [PATCH] run-command.c: fix build warnings on Ubuntu Michael Wookey
2010-01-30 16:43 ` Markus Heidelberg
2011-03-16 3:51 ` Jonathan Nieder [this message]
2011-03-16 5:37 ` [PATCH] run-command: prettify -D_FORTIFY_SOURCE workaround Junio C Hamano
2011-03-16 7:32 ` [PATCH v2] " Jonathan Nieder
2011-03-17 22:34 ` Junio C Hamano
2011-03-16 9:17 ` [PATCH] " Johannes Sixt
2011-03-16 9:25 ` Jonathan Nieder
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20110316035135.GA30348@elie \
--to=jrnieder@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=markus.heidelberg@web.de \
--cc=michaelwookey@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).