Hi Sakari, On Tue 11 Jan 22, 13:38, Sakari Ailus wrote: > Hi Paul, > > On Tue, Jan 11, 2022 at 09:28:12AM +0100, Paul Kocialkowski wrote: > > Hi Sakari, > > > > On Tue 11 Jan 22, 00:48, Sakari Ailus wrote: > > > Don't pack the driver specific struct containing control pointers. This > > > lead to potential alignment issues when working with the pointers. > > > > Thanks for looking into the report and making this fix. > > > > Honestly I was a bit puzzled because I explicitly added the __packed > > to avoid possible holes in the structures that could be problematic > > when using v4l2_ctrl_auto_cluster and I think the problem still stands. > > > > I feel like solving both issues at once would require having the controls > > that belong in the same cluster declared as an array and not individual > > members of the struct. > > > > What do you think? > > No architecture used in Linux requires adding padding between two pointers > to my knowledge --- generally the alignment is at most the size of the > data: otherwise arrays would not work either. Therefore packing isn't > required. I was under the impression that padding may happen in structures generally speaking. Are you saying that because it's pointers, there will most likely be no padding required? Also there's a struct v4l2_ctrl_handler at the end of the struct (not a pointer), maybe that can somehow play a role too and introduce padding? My feeling was that there's no strong guarantee here, so packing the struct would be the safe thing to do. I also don't see how unaligned access can occur in the packed struct in that case (pointers should always offset to something properly aligned, shouldn't they?). Paul -- Paul Kocialkowski, Bootlin Embedded Linux and kernel engineering https://bootlin.com
Attachment:
signature.asc
Description: PGP signature
