netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Mark Rustad <mrustad@gmail.com>
To: Rasmus Villemoes <linux@rasmusvillemoes.dk>,
	"Rustad, Mark D" <mark.d.rustad@intel.com>
Cc: Stanislaw Gruszka <sgruszka@redhat.com>,
	Kalle Valo <kvalo@codeaurora.org>,
	"linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] iwl4965: Enable checking of format strings
Date: Thu, 12 Feb 2015 23:55:47 -0800	[thread overview]
Message-ID: <54DDAE03.4000502@gmail.com> (raw)
In-Reply-To: <87vbj7zb0e.fsf@rasmusvillemoes.dk>

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 2/12/15 2:20 AM, Rasmus Villemoes wrote:
> Rather weak arguments, but I have three of them :-)

Yes, weak. All three.

> (1) If I'm reading some code and spot a non-constant format
> argument, I sometimes track back to see how e.g. fmt_value is
> defined. If I then see it's a macro, I immediately think "ok, the
> compiler is doing type-checking". If it is a const char[], I have
> to remember that gcc also does it in that case (as opposed to for
> example const char*const).

GCC should check in both cases. The case you are replacing was not
const char * const, but only const char *. Still, the compiler really
should check either form, even though theoretically the pointer in the
latter case could be changed, but the initial const value should be a
good indication of what the parameters are expected to be. No real
reason for the compiler not to check it.

> (2) The names of these variables themselves may end up wasting a
> few bytes in the image.

Maybe in a debug image, but they should be stripped from any normal
image. Really not a factor.

> (3) gcc/the linker doesn't merge identical const char[] arrays
> across translation units. It also doesn't consider their tails for
> merging with string literals. So although these specific strings
> are unlikely to appear elsewhere, a string such as "%10u\n" or
> "max\n" couldn't be merged with one of the above.

I haven't checked, but there is no theoretical reason that const char
[] items could not be merged exactly as the literals are. Considering
the boundaries the compiler guys push on optimization, doing such
merging would be tame by comparison (speculative stores make me crazy).
-----BEGIN PGP SIGNATURE-----
Version: GnuPG/MacGPG2 v2.0.20 (Darwin)
Comment: GPGTools - http://gpgtools.org

iQIcBAEBAgAGBQJU3a3+AAoJEDwO/+eO4+5unvMP/jwxA4GmwC0d3VdGsVTJkMVd
zg+jwbkhnMiaEj6uPAwV5LXV/IGyuYFgNjoiNDg9RD3trV9/3YAxdAKw1ffO+PWe
lnmXSxaapLlCTapOsUdXPg88z9muQKrcfhnGyi+jt3BFeccXgtlHLsR0qVa4ddJw
KVHByPg+AlTSNzSnROxHH3UAbxEuZmDy+g+xfbEFLCKNCgtrSX5jGyG2vJIY3lhF
40VIdriUHz1QW4C1YYeJWMKwzml7Kln3u0T5MfDEtDfy6n7hiBhHczEgPjf7dnzd
aY4+VtKTyjPWLRhyDoJfR9maaV9TsYHpheSuUVzAGwvb85wH32ugdfmcW2RPnRfC
n9ThhtWd1WdJJpmq0xhLjc9bc3nrxJO8b2J/GMsT6SjGBhPGaaJSWY37UPhhOJOj
akKkA6QwD0u6Yoc3de7unGsiKWayD7e2k3w3bus+kCSspmyn/OnkzZRc0X3nXd20
suAWNZTalLWioqvI/hyvH3GMZxIuHTJoLRpTm+K7BQs7gBM9pD7OJOpn7XLtk2PM
zPfEj8fAUMV17lzFdBP+M+pGT3HzjWVwTIUgujdA4vL6eqB1W+3fR7kqjUuQYc69
aBaMde//i+HUPzTHZht2qXEb6K9EvSsz/XlhQrtAyu2gYY8PwchdZXH0NbAGqJ9C
4BEAO4HYJijd4vVSNBFO
=utge
-----END PGP SIGNATURE-----

  reply	other threads:[~2015-02-13  7:55 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-02-11 22:51 [PATCH] iwl4965: Enable checking of format strings Rasmus Villemoes
2015-02-12  0:41 ` Rustad, Mark D
2015-02-12 10:20   ` Rasmus Villemoes
2015-02-13  7:55     ` Mark Rustad [this message]
2015-02-13 10:39       ` Rasmus Villemoes
2015-02-13 11:20         ` David Laight
2015-02-13 12:04           ` Rasmus Villemoes
     [not found] ` <1423695069-23436-1-git-send-email-linux-qQsb+v5E8BnlAoU/VqSP6n9LOBIZ5rWg@public.gmane.org>
2015-04-28 16:19   ` Kalle Valo
2015-04-28 17:49     ` Stanislaw Gruszka

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=54DDAE03.4000502@gmail.com \
    --to=mrustad@gmail.com \
    --cc=kvalo@codeaurora.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=linux@rasmusvillemoes.dk \
    --cc=mark.d.rustad@intel.com \
    --cc=netdev@vger.kernel.org \
    --cc=sgruszka@redhat.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).