Sparse Question
Quoted 9 lines · Harvey Harrison
Quoted 5 lines · Harvey Harrison
Hi Al,
Further to eliminating some of the trivial sparse noise in a kernel build, I just can't seem to understand what sparse is warning about:
I should have mentioned, the other block of warnings comes from
drivers/media/video/videodev.c....againinitializing arrays of IOCTLs
1 ? 0 : x
is not valid in contexts where C requires integer constant expressions. Index in static array initializer is one of those.
gcc allows it, but its extensions in that area are inconsistent, to say the least - basically, it goes with "if optimizer can fold that into constant with this set of options, it will be accepted". With very weird boundary between accepted and not accepted (as in "reorder arguments of +, and what had been recognized as constant is not recognized anymore").
sparse doesn't even try to duplicate that set of bugs. We _could_ try to go for a more or less reasonable subset (e.g. ?: with integer constant expression as the first argument and integer constant expression as the second or the third resp., depending on the value of the first one, similar for && and ||), but I'm not all that sure that it's worth doing.
The fact is, use of what we have for _IOC in such contexts is not just a gccism, it's ill-defined one. I suspect that the right solution is to sanitize _that_...
FWIW, why not simply put division by 0 into the branch that shouldn't be reached instead of using a variable that doesn't exist and would blow at ld(1) time? I.e. go with
#define _IOC_TYPECHECK(t) \
((sizeof(t) == sizeof(t[1]) && \
sizeof(t) < (1 << _IOC_SIZEBITS)) ? \
sizeof(t) : 1/0)instead. I'd say that trading a pretty name in linker stderr for compiler error that shows exact location in the source would be a good bargain...
Linus, would you object against that in post-2.6.25?
Quoted 39 lines · Al Viro
Quoted 9 lines · Harvey Harrison
Quoted 5 lines · Harvey Harrison
Hi Al,
Further to eliminating some of the trivial sparse noise in a kernel build, I just can't seem to understand what sparse is warning about:
I should have mentioned, the other block of warnings comes from
drivers/media/video/videodev.c....againinitializing arrays of IOCTLs1 ? 0 : x
is not valid in contexts where C requires integer constant expressions. Index in static array initializer is one of those.
gcc allows it, but its extensions in that area are inconsistent, to say the least - basically, it goes with "if optimizer can fold that into constant with this set of options, it will be accepted". With very weird boundary between accepted and not accepted (as in "reorder arguments of +, and what had been recognized as constant is not recognized anymore").
sparse doesn't even try to duplicate that set of bugs. We _could_ try to go for a more or less reasonable subset (e.g. ?: with integer constant expression as the first argument and integer constant expression as the second or the third resp., depending on the value of the first one, similar for && and ||), but I'm not all that sure that it's worth doing.
The fact is, use of what we have for
_IOCin such contexts is not just a gccism, it's ill-defined one. I suspect that the right solution is to sanitize _that_...FWIW, why not simply put division by 0 into the branch that shouldn't be reached instead of using a variable that doesn't exist and would blow at ld(1) time? I.e. go with
#define _IOC_TYPECHECK(t) \ ((sizeof(t) == sizeof(t[1]) && \ sizeof(t) < (1 << _IOC_SIZEBITS)) ? \ sizeof(t) : 1/0)
Quoted 5 lines
instead. I'd say that trading a pretty name in linker stderr for compiler error that shows exact location in the source would be a good bargain...
Linus, would you object against that in post-2.6.25?
Sorry, maybe I'm thick, but how does _IOC_TYPECHECK get pulled into the _IOC_NR use?
Harvey
Quoted 3 lines · Harvey Harrison
Sorry, maybe I'm thick, but how does
_IOC_TYPECHECKget pulled into the_IOC_NRuse?
Nevermind, I see it now, I was just hung up on the IOC_NR part.
Harvey
PATCH 1/1 asm-generic: suppress sparse warning in ioctl.h
In reply to Al Viro1 ? 0 : x
is not valid in contexts where C requires integer constant expressions. Index in static array initializer is one of those.
Instead of using a non-existant extern function, use 1/0 as the guard expression to avoid using a gcc-ism. IOC_TYPECHECK gets pulled into some static array initializations where this is not valid.
---
I've confirmed this patch 'fixes' the large blocks of sparse warnings in static array initializers.
include/asm-generic/ioctl.h | 4 +--- 1 files changed, 1 insertions(+), 3 deletions(-)
include/asm-generic/ioctl.h +1 -3
@@ -47,12 +47,10 @@ ((nr) << _IOC_NRSHIFT) | \ ((size) << _IOC_SIZESHIFT)) -/* provoke compile error for invalid uses of size argument */ -extern unsigned int __invalid_size_argument_for_IOC;#define _IOC_TYPECHECK (t) \ ((sizeof(t) == sizeof(t[1]) && \ sizeof(t) < (1 << _IOC_SIZEBITS)) ? \ - sizeof(t) : __invalid_size_argument_for_IOC) + sizeof(t) : 1/0)/* used to create numbers */ #define _IO (type,nr) _IOC(_IOC_NONE,(type),(nr),0)
Signature · 1 line
1.5.5.rc1.135.g8527