Message-ID: <20120308025138.3caf84b2@wrlaptop>
Date:	Thu, 8 Mar 2012 02:51:38 -0600
From:	Peter Seebach <peter.seebach@...driver.com>
To:	Ingo Molnar <mingo@...e.hu>
CC:	Peter Zijlstra <peterz@...radead.org>,
	Arnaldo Carvalho de Melo <acme@...hat.com>,
	Anton Blanchard <anton@...ba.org>, <paulus@...ba.org>,
	<dsahern@...il.com>, <fweisbec@...il.com>,
	<yanmin_zhang@...ux.intel.com>, <emunson@...bm.net>,
	<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] perf: Incorrect use of snprintf results in SEGV

On Thu, 8 Mar 2012 08:34:54 +0100
Ingo Molnar <mingo@...e.hu> wrote:

> But the self-justification, as outlined in the mail I replied 
> to, is brutally wrong,

Consider, if you will, an interface which has both snprintf_needed()
and snprintf_written().

Under the hood, any reasonably sane implementation of these functions
needs to have at least one function with the semantics of
snprintf_needed.  APIs which omit that create baroque monstrosities on
the (admittedly relatively rare) occasions when people try to get things
right.

Here, I think, the habit of C programmers of not writing trivial
five-line wrappers because "you could just write that, it's only five
lines", rather bites us.  It might have been better to put both of
these designs in the spec, but at the time when the decision was
originally made, it was clear that there was a need for *a*
length-limited version of snprintf, and if you're only going to have
one, there are significant technical advantages to making the one that
makes the other trivial to provide if you'd prefer it.  It may be
easy to misuse snprintf, but it's nearly impossible to use sprintf_s
correctly if you actually care about your data.

> When designing APIs it is of utmost importance how average 
> developers intuitively *think* it works - not how the designer 
> thinks it should work ...

There is perhaps a subtle clash between the Principle of Least
Astonishment and the Spirit of C.  C has often chosen design decisions
that are intuitively appealing to skilled programmers who have heard
the rationale for them, but which may surprise average programmers or
programmers relying purely on intuition.

I usually think the right thing to do in designing an API is to
provide the smallest and cleanest spanning set possible, and trust
users to know whether they need wrapper functions to provide more
congenial semantics.

The snprintf() interface might well have been done differently if
there'd been any way to get data on how it would be used before it was
specified.  At the time, though, I think the only available criterion
was the observation that one specification could be trivially
implemented in terms of the other.  I suspect, though, that even with
that information, the C committee's response would have been to provide
the function which could predictably produce correct output when used
correctly, rather than the function which couldn't.

We did look at the existing practice, and we heard feedback from people
on what they found with it.  Multiple people present had been badly
burned by one or another API that did not provide a way to find out how
much space was needed.  None of us had, at the time, encountered
this problem with the snprintf() specification, or if we had, it seemed
obvious enough that the fix was trivial.  (Possibly even a Simple
Matter of Programming.)

It may be a decision which produced problems in retrospect, but I think
it was a pretty reasonable decision at the time.

... And I say this even noting that I have at least one hunk of code
which I'm pretty sure makes this bad assumption.

-s
-- 
Listen, get this.  Nobody with a good compiler needs to be justified.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists