Not logged in
Log in now
Create an account
Subscribe to LWN
LWN.net Weekly Edition for December 5, 2013
Deadline scheduling: coming soon?
LWN.net Weekly Edition for November 27, 2013
ACPI for ARM?
LWN.net Weekly Edition for November 21, 2013
Posted Oct 6, 2011 11:11 UTC (Thu) by PaXTeam (subscriber, #24616)
Posted Oct 6, 2011 14:07 UTC (Thu) by epa (subscriber, #39769)
solving the "let's make all constifiable variables const" problem is not possible with patching due to the sheer amount of patching needed.
I guess the answer would be to generate the patch automatically against a given source tree. Then even if only a fraction of the total patch can be applied at a time, it is possible to keep trying. Clearly if it takes a week of manual work to create the patch it's never going to be practical to get it in.
Posted Oct 6, 2011 19:41 UTC (Thu) by PaXTeam (subscriber, #24616)
the first problem you'll hit with manual patching is that it takes a lot of time to check every variable instance and their use to determine whether the given type can be made read-only or not (or you'll have to settle on individual variables only), the plugin approach is already a godsend for that reason (in the past another route was tried with coccinelle but that didn't work out too well).
the second problem is patching a given tree *and* forward porting the changes to a newer kernel, including redoing the analysis in the first step since new uses of a given type can come and go any time, meaning that the constifiable property of a type can change either way over time. the plugin approach is again the most efficient way of tracking these kind of changes.
last but not least, once you have such a plugin that can determine when to do constification, it's a very natural step to actually do it from the same plugin (it's like one extra line of code to set TREE_READONLY on the type) at which point one's enthusiasm to maintain a huge patch quickly evaporates ;).
now this was the producer side, but the consumer side is not any better unfortunately. if you read those lkml threads when constification patches were submitted, you'll realize that different developers expected them in different (and conflicting) ways (broken up by subsystem or maintainer or directory or structure type, etc), putting a huge burden on the producer side as if creating the patches wasn't consuming enough time already. then there's an issue with enforcing policy as well, seemingly noone takes constification or checkpatch.pl seriously enough as i keep seeing patches go in all the time that simply don't bother to constify structures.
all in all, while patching the source code is surely a noble goal, my and other people's lives are too short for it...
Posted Oct 7, 2011 6:26 UTC (Fri) by Lionel_Debroux (subscriber, #30014)
I myself gave up trying to push upstream the constification of snd_pcm_ops, which represented ~10% of the size of the constification part of the PaX patch, because there were so many instances to check...
Could the plugin be modified to print warnings about pointers to non-const instances being passed to functions that take pointers to const instances ? (if enabled by some argument to the plugin, because upon introduction of that feature and for quite some time afterwards, there will be thousands of occurrences...)
For instance, this could have helped against addition of mutable instances of backlight_ops being added after backlight_device_register() was modified to take a "const struct backlight_ops *ops" argument...
Finding functions that take pointers to mutable instances while they could take pointers to const instances would be harder, wouldn't it be ?
Posted Oct 7, 2011 8:22 UTC (Fri) by PaXTeam (subscriber, #24616)
it's surely possible but i'm not sure this is what you really want as such typecasts are allowed by C and extensively used everywhere. now i assume you'd really want this detection for ops structures only in which case the problem is how the plugin would know of them (i assume you wouldn't want to use the current 'constify by default' approach). you could make use of the do_const attribute (without calling constify_type) then in the FINISH_TYPE callback you can check whether the pointed to structure is already const or not (and has the do_const attr, although on second thought, i forget now whether attrs are carried with types or have to be acted upon in the attr callback).
with all that said, i would not go the 'check the pointer' route but rather i'd constify the variable instances instead (but not function arguments) and then rely on gcc's existing warning system (if you take the address of a const object to initialize a ptr to the non-const type then you'll get a warning).
Posted Oct 7, 2011 15:24 UTC (Fri) by vonbrand (subscriber, #4458)
That there are lots of places that would require patching, and that the patch covering all of the different variables is large, isn't reason enough to just don't send patches fixing it piecewise upstream. Presumably there are reasons for not accepting said patches (probably much lessened today by using git).
Copyright © 2013, Eklektix, Inc.
Comments and public postings are copyrighted by their creators.
Linux is a registered trademark of Linus Torvalds