Re: [PATCH] iio: adc: at91-sama5d2_adc: fix up casting in at91_adc_read_info_raw()

Dan Carpenter
2018-07-09T11:41:07Z

On Mon, Jul 09, 2018 at 02:22:40PM +0300, Eugen Hristev wrote:
>
>
> On 09.07.2018 14:06, Dan Carpenter wrote:
> > This code is problematic because we're supposed to be writing an int but
> > we instead write to only the high 16 bits. This doesn't work on big
> > endian systems, and there is a potential that the bottom 16 bits are
> > used without being initialized.
>
> Hi Dan,
>
> Thanks for the patch.
> Please correct me if I'm wrong, the caller of this function should mask out
> the unused bits w.r.t. the channel spec ?
>
> Indeed there may be an issue if we actually write the data to the wrong 16
> bit part of the 32 bit integer.
>
> Would be safer to check for the endianess and write the proper part of the
> int ? (macros that do the magic for us - cpu_to_le etc.), or we rely on the
> compiler to do it for us as it looks in your code ?
>
> Another option is to pass the int directly and do the ugly task inside the
> read_position/pressure functions, I am not sure which one looks better
>

To be honest, I'm just doing static analysis. I'm not very familiar
with the subsystem and I don't know the answers to your questions.

The code as it's written now doesn't make sense. I looked at code like
ntc_adc_iio_read() where it's called like so:

int raw, uv, ret;

ret = iio_read_channel_raw(channel, &raw);

If we only write to the high 16 bits then the low 16 bits of "raw" are
uninitalized.

regards,
dan carpenter