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 -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
