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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 20 Sep 2017 10:34:43 +0200, Jani Nikula <jani.nikula@xxxxxxxxx> wrote:

On Tue, 19 Sep 2017, Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx> 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux