Messages in this thread |  | | Date | Tue, 27 Nov 2001 07:14:20 +0000 | From | Nathan Myers <> | Subject | Re: [RFC] [PATCH] omnibus header cleanup, certification |
| |
On Mon, Nov 26, 2001 at 10:23:24PM -0800, H. Peter Anvin wrote: > Nathan Myers wrote: > > Please review at least the portion that you are responsible for, > > and verify that the changes are safe and cannot change the semantics > > of otherwise-correct code that uses the affected macros. (That will > > be easy, they're all trivial parenthesizations. It seems worth noting > > here that in the C grammar, "-1" is not a numeric literal, but a unary > > expression.) > > It's also worth noting that there is nothing that it can get confused > with and still have a compilable expression.
Not true, in principle. E.g.
#define foo -1 ... char f(char* a) { return foo[a]; } // -a[1] or a[-1]?
Such an expression should, of course, be taken out and shot (although it's far less bad than "(x = x++ % y)" that appeared in a macro until very recently).
The only compound expressions that are strictly safe have one of the forms
(...) a.b a->b a[...]
because those are the only operators at the top precedence level. Even with those, an ill-defined macro that replaces "a" or "b" (such as are found in de4x5.h) can cause confusion. > I don't believe the unary-expression patches are necessary.
One #define found in a mips header is identical to another that was in an i386 header and already caused a bug. That was what motivated me to see if there were other cases, too.
If this patch is accepted, I will be motivated to report more subtle sources of bugs, which remain numerous. If we can't even be bothered with the basics, who can care about subtleties?
> They are, > of course, harmless, except for the fact that my eyes glazed over > staring at page after page of these, which very few actual potential (!) > bugs (there were a couple, like the iopage+ ones...)
I assure you it was no less boring making the patch. I hope not to have to keep re-making it as the existing patch rots.
The point of making a wholesale change including even "unnecessary" definitions that are "unlikely" to result in bugs (such as "-1"), is to establish a convention so that people who don't necessarily understand all the nuances will do the right thing anyhow. There is certainly plenty of code from such people in the kernel, heaven help us.
Nathan Myers ncm at cantrip dot org - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
|  |