[Intel-gfx] [PATCH v6 3/3] drm/i915: Make i915_modparams members const
Joonas Lahtinen
joonas.lahtinen at linux.intel.com
Wed Sep 20 12:13:27 UTC 2017
On Wed, 2017-09-20 at 15:06 +0300, Joonas Lahtinen wrote:
> On Wed, 2017-09-20 at 11:34 +0300, Jani Nikula 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.
> >
> > 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.
>
> Here's the snippet to describe the three discussed behaviors. Michal's
> code seems to do the right thing:
>
> https://gcc.godbolt.org/g/6MCNC3
>
> We just need to make the write function stand out more and have a
> kerneldoc for it.
>
Umm, and when I don't typo a missing "&", it's more obvious (now with
--Wall -Wextra -Werror):
https://gcc.godbolt.org/g/HszLnw
Regards, Joonas
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
More information about the Intel-gfx
mailing list