Re: [PATCH 1/3] Drivers: hv: Support the newly introduced KVP messages in the driver

On Fri, Mar 16, 2012 at 01:14:07PM +0000, KY Srinivasan wrote:

Dan,
I am trying to understand your concern here:
You will agree with me that the current code does not overflow the 
buffer since the output is limited to MAX bytes and that is how 
big the output buffer is sized. Now, as I pointed out earlier, if the
string to be converted were to fully occupy the MAX bytes or even be
larger than MAX bytes (no buffer overflow here since we limit the
conversion to MAX bytes), we will get a malformed packet that will be
sent to the host since the size field of the message would exceed
the protocol specified size limit. I was merely pointing out that in
this case, I would rather have the host reject the message than send
a truncated Key/Value string (if we were to ever get invalid key or
value strings). 

Sending malformed packets is a bad idea.

Normally, if you handle the error as close to the cause of the error
as possible then it makes everything easier to read.  In this case
especially, it's so easy to catch any errors.  If you decide to go
ahead and send the malformed message, at least put a comment.

When I read code, I spend all my time looking for what it does
wrong.  So when code doesn't have any error handling and sets
keylen = -EINVAL then sure, it's fewer lines of code to read, but
it doesn't make my life easier.  Readable code has obvious error
handling.

Introducing off by one errors is an especially bad idea.  It doesn't
matter how harmless they are, because they end up getting copy and
pasted.  I don't know how to make it any clearer than that.  :/
Never never never do that!

regards,
dan carpenter