lkml.org 
[lkml]   [2011]   [Nov]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] i2c/busses: (mpc) Add support for SMBUS_READ_BLOCK_DATA
From
Date
> > That's the point. Your patch doesn't check if the length is valid. You
> > rely on the caller to know no more than 32 bytes can be transferred. It
> > shouldn't limit the subfunction to transfer more than 32 bytes if
> > hardware can support it by multiple transactions. If not, print out an
> > error message.
> >
>
> It is customary for kernel functions to only validate parameters
> wherever a value enters or leaves the kernel, or at least a logical
> boundary. Anything else would blow up the kernel size to a point where
> it would be unusable. The patch checks if the block length received from
> the SMBus slave is correct, which is exactly what it is supposed to do.
>
> If you look closely, you may realize that the mpc_read also doesn't
> validate of any of the other parameters. Are you going to request that
> such validations be added as well ? How about the other functions in
> this driver ? Should each function also validate each of its
> parameters ? If not, where do you draw the line ?
>
> Besides, the caller is the SMBus block read function, which does know
> that SMBus block reads are limited to 32 bytes (plus length).
>

I am going to let it go for now. Obviously your patch is working when
length=1. Probably it will never be called with length other than 1 for
SMBus block read. It would be nicer if you could put a comment there
just in case in the future someone runs into a case where length+=byte
causes a problem.

York





\
 
 \ /
  Last update: 2011-11-17 20:27    [W:0.089 / U:0.276 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site