On 10/10/23 17:47, Jonathan Cameron wrote:
On Tue, 10 Oct 2023 15:56:22 +0300
Matti Vaittinen <mazziesaccount@xxxxxxxxx> wrote:
On 10/10/23 13:04, Jonathan Cameron wrote:
On Fri, 6 Oct 2023 14:10:16 +0300
Matti Vaittinen <mazziesaccount@xxxxxxxxx> wrote:
Hi Again Jonathan.
On 10/5/23 18:30, Jonathan Cameron wrote:
On Tue, 3 Oct 2023 12:49:45 +0300
Matti Vaittinen <mazziesaccount@xxxxxxxxx> wrote:
...
Other option I see is to just error out if available_scan_masks array is
given with larger than one 'long' wide masks and worry things when this
breaks.
That would kick the problem into the long grass.
Well, not 100% sure I interpret the idiom correctly ;) In any case, I'd
say this would indeed postpone dealing with the problem to the future.
It does indeed mean that! Sorry bad habit to use idioms in discussions like
this.
To the point we actually seem to have a problem. The "long grass" as if
hiding the problem is something we can avoid by adding something like:
if (masklength > 32 && idev->available_scan_masks) {
/*
* Comment mowing the long grass.
*/
dev_err( ...);
return -EINVAL;
}
to the device registration.
...
iio_dev->available_scan_masks = (unsigned long *)available_masks;
If we put such an example into the dummy / example driver then that might
act to avoid us getting bugs in future + test the fix you have above and
related.
Well, at least it shouldn't hurt to have some example - although I'm
still tempted to use the "long grass" - option ;)
That is probably a good idea for now. Though we are carrying other infrastructure
to support this eventually and it feels weird to error out on it whilst we have
code to support it (assuming that terminator is long enough).
I agree. I think I won't use the bitmap_empty() - because I feel it is
unsafe. I'll leave the *av_masks check as it is implemented in
iio_scan_mask_match() for now. Eg:
... const unsigned long *av_masks ...
while (*av_masks) {
...
av_masks += BITS_TO_LONGS(masklength);
}
This will fail if mask is longer than unsigned long - and if we have
masks with zero bits worth a leading long. Still, this won't overflow
and it also works for masks which are wider than long but do not have
the leading bits zeroed. Balanced act of safety and functionality.
This should allow us to safely do:
if (masklength > 32 && idev->available_scan_masks) {
/*
* Comment mowing the long grass.
*/
dev_warn( ...);
}
without returning the error.
Not perfect, but should be safe and also adds a warning if someone
trusts the multi-long masks to work.
Yours,
-- Matti
--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland
~~ When things go utterly wrong vim users can always type :help! ~~
