On 3/27/07, Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
On Mon, 26 Mar 2007, Derek M Jones wrote:
> This is a useful check. However, you need to make sure that
> cases where the presence of () have no impact are not flagged
> if the parameter is always used in such contexts. Otherwise the
> noise can be excessive.
Indeed. In the kernel, for example, we sometimes use nested macros for the
complex cases, and there one of the cases is where the "outer" or inner
macro adds all required brackets. Ie something like
#define ALIGN(x,a) __ALIGN_MASK(x,(typeof(x))(a)-1)
#define __ALIGN_MASK(x,mask) (((x)+(mask))&~(mask))
where ALIGN() uses 'x' without any brackets, because it knows the inner
macro is safe (and a comma expression must have had any brackets of its
own, otherwise it wouldn't have parsed as a single macro argument anyway!)
Hmm it looks like a 'bracket or comma' check wouid correcly ignore
this particular example.
It is obvious however that I need to think about this a bit more.
> > 2) For the macro argument checking would it be better if I checked for
> > either a bracket or low precedence operator on both sides of the
> > argument ? like either a comma or a type of assignment operator? it
> > would not be a foolproof check then but would seem more sensible to me,
> > and might be more acceptable to users.
>
> Don't make the assumption that everybody knows the precedence
> of binary operators. An experiment I ran last year with experienced
> developers (average over 10 years) threw up some unexpected results.
> See: http://www.knosof.co.uk/cbook/accu06a.pdf
Yep I am guilty of getting them wrong before.
Yeah. The only exception is literally the comma expression, because of the
syntax of macro expansion itself (ie a comma would break a macro argument,
unless it is bracketed). Pretty much all other precedence rules can pretty
much be expected to be confused by somebody.
I didn't look into it but I was thinking that something that evaluated
into a comma, after the initial macro expansion would still be
passible/an issue.
Thanks for the feedback guys. I guess I will keep tinkering with it.
Darren J.
-
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
