git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jonathan Nieder <jrnieder@gmail.com>
To: Jeff King <peff@peff.net>
Cc: Felipe Contreras <felipe.contreras@gmail.com>, git@vger.kernel.org
Subject: Re: [PATCH 1/2] inline constant return from error() function
Date: Mon, 12 May 2014 11:44:26 -0700	[thread overview]
Message-ID: <20140512184426.GO9218@google.com> (raw)
In-Reply-To: <20140506151441.GA25768@sigill.intra.peff.net>

Hi,

Jeff King wrote:
> On Mon, May 05, 2014 at 05:29:38PM -0400, Jeff King wrote:

>> I cannot think of any other way to make the compiler aware of the
>> constant value, but perhaps somebody else is more clever than I am.
>
> This came to me in a dream, and seems to work.

Clever. :)  Thanks for thinking it up.

[...]
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -331,7 +331,11 @@ extern void warning(const char *err, ...) __attribute__((format (printf, 1, 2)))
>   * using the function as usual.
>   */
>  #if defined(__GNUC__) && ! defined(__clang__)
> -#define error(...) (error(__VA_ARGS__), -1)
> +static inline int const_error(void)
> +{
> +	return -1;
> +}
> +#define error(...) (error(__VA_ARGS__), const_error())

I wish we could just make error() an inline function that calls some
print_error() helper, but I don't believe all compilers used to build
git are smart enough to inline uses of varargs.  So this looks like
the right thing to do.

I kind of wish we weren't in so much of an arms race with static
analyzers.  Is there some standard way to submit our code as "an idiom
that should continue not to produce warnings" to be included in a
testsuite and prevent problems in the future?

Thanks,
Jonathan

  parent reply	other threads:[~2014-05-12 18:44 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-04  6:12 [PATCH 0/3] Fix a ton of compiler warnings Felipe Contreras
2014-05-04  6:12 ` [PATCH 1/3] Revert "make error()'s constant return value more visible" Felipe Contreras
2014-05-05  5:49   ` Jeff King
2014-05-05  5:45     ` Felipe Contreras
2014-05-05  6:02       ` Jeff King
2014-05-05  6:14         ` Felipe Contreras
2014-05-05  6:29           ` Jeff King
2014-05-05  7:30             ` Felipe Contreras
2014-05-05 21:29               ` Jeff King
2014-05-06 15:14                 ` [PATCH 1/2] inline constant return from error() function Jeff King
2014-05-06 22:29                   ` Junio C Hamano
2014-05-07  3:02                     ` Jeff King
2014-05-11 17:22                       ` Junio C Hamano
2014-05-12 18:13                         ` Jeff King
2014-05-11  7:13                     ` Felipe Contreras
2014-05-12 18:44                   ` Jonathan Nieder [this message]
2014-05-12 18:56                     ` Jeff King
2014-05-06 15:17                 ` [PATCH 2/2] let clang use the constant-return error() macro Jeff King
2014-05-04  6:12 ` [PATCH 2/3] Revert "silence some -Wuninitialized false positives" Felipe Contreras
2014-05-04  6:12 ` [PATCH 3/3] Silence a bunch of format-zero-length warnings Felipe Contreras
2014-05-04 19:01   ` brian m. carlson
2014-05-04 20:13     ` Felipe Contreras
2014-05-05  5:21     ` Jeff King
2014-05-07 18:19       ` Junio C Hamano
2014-05-07 20:05         ` Heiko Voigt
2014-05-07 20:31           ` Junio C Hamano

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=20140512184426.GO9218@google.com \
    --to=jrnieder@gmail.com \
    --cc=felipe.contreras@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    /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).