On Sat, Jul 29, 2023 at 12:46:18PM +0100, Jonathan Cameron wrote: > On Mon, 24 Jul 2023 14:02:02 +0300 > Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote: ... > > + * Note, when @s is 0, the alignment @a is added to the sizeof(*(@p)) > > + * and the result, depending on the @a, may be way off the initial size. > > How often is this true? A quick and dirty grep suggests at least 2 so perhaps > worth retaining the old behaviour. You mean that the sizeof(_some_grepped_struct_) is much less than an alignment in those uses? > Can we take that into account? Maybe something like > > #define opaque_struct_size(p, a, s) ((s) ? size_add(ALIGN(sizeof(*(p)), (a)), (s)): sizeof(*p)) (s) will be evaluated twice, not good. So, not in this form. > Or do it at the call site below. Looks much better to me. ... > if (sizeof_priv) > alloc_size = opaque_struct_size(iio_dev_opaque, IIO_DMA_MINALIGN, sizeof_priv); > else > alloc_size = sizeof(struct iio_dev_opaque); Right. ... > > - indio_dev->priv = (char *)iio_dev_opaque + > > - ALIGN(sizeof(struct iio_dev_opaque), IIO_DMA_MINALIGN); > > + indio_dev->priv = opaque_struct_data(iio_dev_opaque, IIO_DMA_MINALIGN); > > Would have been safer if original code set this to NULL if > sizeof_priv == 0 Yeah, original code and proposed change has no difference in this sense. > A driver doing that should never have used iio_priv() but nicer if it was > NULL rather than off the end of the allocation. Agree. But looking at the above, I would rather see that in a form of if (...) priv = opaque_struct_data(...); else priv = NULL; -- With Best Regards, Andy Shevchenko
