Messages in this thread | | | Date | Thu, 10 Dec 2009 19:24:21 +0100 | From | Emese Revfy <> | Subject | Re: [PATCH 28/31] Constify struct super_operations for 2.6.32 v1 |
| |
Al Viro wrote: > On Wed, Dec 09, 2009 at 01:24:34AM +0100, Emese Revfy wrote: > >> If constifying the function pointer fields reduces readability, >> what would you say for turning then into typedefs, something like this: >> >> typedef int (* super_ops_statfs) (struct dentry *, struct kstatfs *); >> struct super_operations { >> ... >> const super_ops_statfs statfs; >> ... >> }; > > Even worse, since one has to go back to typedef to figure out WTF is > going on. > >>> Moreover, you *still* are not >>> covering the real policy - these suckers should be statically allocated, >>> not just never modified. >> If the super ops are allocated on the stack then they will be overwritten >> during later syscalls and will eventually crash the system on a future >> dereference, that is, this kind of problem manifests during development. >> >> If the super ops are allocated by kmalloc/etc, then they will have to be >> explicitly initialised by writing to specific fields, my patch would prevent >> that. >> >> So in the end the programmer is forced to allocate and initialise super ops >> statically. > > ... unless they go ahead and use memcpy(), etc. > > What you really want is > * no conversions to any other pointer types for pointers to it > and to any aggregate types containing it > * no conversions from any other pointer types for the same set of > types > * all objects of that type have static storage duration > * no lvalues of that type are modifiable > > Which is not a job for C compiler. Yes, (4) means that memcpy() et.al. > give undefined behaviour. And you get fsck-all satisfaction from knowing > that, since C compiler is not going to warn you about it. sparse might, > if we teach it to do so. Preferably - with minimal intrusiveness of > syntax being used.
I think, all these instruments (constification, sparse, etc.) are not for preventing a programmer from circumventing the policy (that's impossible), but to make it easy for the reviewer to notice it when he does so. My patch achieves this in a very simple way for the currently uncovered case of dynamically allocated ops structures. -- Emese
| |