On Wed, 22 Jul 2020 13:28:35 +0200 Pavel Machek <pavel@xxxxxxx> wrote: > Hi! > > > From: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx> > > > > commit 5c49056ad9f3c786f7716da2dd47e4488fc6bd25 upstream. > > > > One of a class of bugs pointed out by Lars in a recent review. > > iio_push_to_buffers_with_timestamp assumes the buffer used is aligned > > to the size of the timestamp (8 bytes). This is not guaranteed in > > this driver which uses an array of smaller elements on the stack. > > I don't see documentation explaining alignment issues with > iio_push_to_buffers_with_timestamp(). Perhaps comment near that > function should explain that? Hi Pavel, Agreed. It's a subtle corner case (hence we missed it for years) so absolutely needs documenting. The nasty part is that we don't control the expectations of the consumers who get the data from that interface. They may make the reasonable assumption that they aren't getting unaligned data, particularly given the effort we go to in ensuring natural alignment of elements within the buffer. It's a moderately fast path so any tricks with realigning the data aren't sensible either. > > And as it seems to be common problem, perhaps > iio_push_to_buffers_with_timestamp should check alignment of its > arguments? It should indeed check this. But... The reality is that lots of platforms are fine with the alignment not being enforced. So far we have precisely one confirmed report of the issue. Until we have fixed all the users I'm not keen to add a check that will be seen to 'break' existing working systems. It's taking a while to get all these reviewed so I'm picking them up as they get sufficient eyes on them. A few drivers are more fiddly to do so we don't yet have patches on the list. I was thinking to do the documentation update and a check enforcing it in one go, but perhaps given the slow nature of getting all the users fixed we should look to document now and enforce later? Jonathan > > Thanks, > Pavel
