Re: [PATCH v6 3/3] drm/i915: Make i915_modparams members const

[Joonas Lahtinen <joonas.lahtinen@linux.intel.com>]

On Wed, 2017-09-20 at 11:34 +0300, Jani Nikula wrote:
> On Tue, 19 Sep 2017, Michal Wajdeczko <michal.wajdeczko@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.

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx