[Intel-gfx] [PATCH v6 3/3] drm/i915: Make i915_modparams members const
Michal Wajdeczko
michal.wajdeczko at intel.com
Wed Sep 20 09:54:03 UTC 2017
On Wed, 20 Sep 2017 10:34:43 +0200, Jani Nikula <jani.nikula at intel.com>
wrote:
> On Tue, 19 Sep 2017, Michal Wajdeczko <michal.wajdeczko at intel.com> wrote:
>> We should discourage developers from modifying modparams.
>> Introduce special macro for easier tracking of changes done
>> in modparams and enforce its use by defining existing modparams
>> members as const. Note that defining whole modparams struct
>> as const makes checkpatch unhappy.
>
> Checkpatch is the least of all reasons to not make the modparams struct
> const.
>
> We can get away with having some fields (such as device info within
> dev_priv) const, even if that's dubious.
>
> IIUC modifying const data is undefined behaviour at best, could cause
> subtle bugs through compiler optimizing reads of the data away because
> it assumes no modifications, and the data gets placed in rodata at
> worst.
Note that we're forcing modparams to be stored in dedicated section:
#define __read_mostly __attribute__((__section__(".data..read_mostly")))
>
> I kinda like the union trick in this patch, but IMO we need to double
> check what the standard says about it. Making the fellow developers
> check the standard is always a bad sign, even if it turns out to be fine
> after all.
ISO/IEC 9899: 6.5.2.3
If the member used to access the contents of a union object is not the same
as the member last used to store a value in the object, the appropriate
part
of the object representation of the value is reinterpreted as an object
representation in the new type as described in 6.2.6 (a process sometimes
called "type punning").
Michal
More information about the Intel-gfx
mailing list