RE: [PATCH 2/4] Drivers: hv: Support the newly introduced KVPmessages in the driver

[KY Srinivasan]

> -----Original Message-----
> From: Dan Carpenter [mailto:dan.carpenter@xxxxxxxxxx]
> Sent: Monday, March 12, 2012 1:22 AM
> To: KY Srinivasan
> Cc: gregkh@xxxxxxxxxxxxxxxxxxx; ohering@xxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; virtualization@xxxxxxxxxxxxxx; Alan Stern;
> devel@xxxxxxxxxxxxxxxxxxxxxx
> Subject: Re: [PATCH 2/4] Drivers: hv: Support the newly introduced KVP
> messages in the driver
> 
> On Sun, Mar 11, 2012 at 08:53:57PM +0000, KY Srinivasan wrote:
> >
> >
> > > -----Original Message-----
> > > From: Dan Carpenter [mailto:dan.carpenter@xxxxxxxxxx]
> > > Sent: Sunday, March 11, 2012 2:49 PM
> > > To: KY Srinivasan
> > > Cc: gregkh@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> > > devel@xxxxxxxxxxxxxxxxxxxxxx; virtualization@xxxxxxxxxxxxxx;
> ohering@xxxxxxxx;
> > > Alan Stern
> > > Subject: Re: [PATCH 2/4] Drivers: hv: Support the newly introduced KVP
> > > messages in the driver
> > >
> > > On Sun, Mar 11, 2012 at 04:56:06PM +0000, KY Srinivasan wrote:
> > > > > Probably that's not enough to make a difference and we'd need to
> > > > > introduce a new function.
> > > > >
> > > > > Btw I don't know if utf16s_to_utf8s() counts the NUL char or not.
> > > > > It feels like maybe we could end up with ->value_size equal to
> > > > > HV_KVP_EXCHANGE_MAX_VALUE_SIZE + 1.
> > > >
> > > > The MAX value is set to accommodate the maximum string that will ever
> > > > be handled including the string terminator. The function utf16s_to_utf8s()
> > > > returns the converted string length but the returned length does not
> > > > include the string terminator (like strlen), hence the "+1".
> > > >
> > >
> > > sprintf() and friends copy the NUL terminator but utf16s_to_utf8s()
> > > doesn't so the code isn't right and it does seem like maybe we could
> > > end up with a ->value_size equal to HV_KVP_EXCHANGE_MAX_VALUE_SIZE
> +
> > > 1.
> >
> > You are right in that utf16s_to_utf8s() does not copy the string
> > terminator. This is not an issue in this case since the buffer for
> > the utf8 string is zeroed out to begin with (this memory was
> > allocated using kzalloc()). The return value of the
> > utf16s_to_utf8s() is the length of the utf8s string as what would
> > be returned by strlen.
> 
> 
> There is no strlen() involved...  It returns the number of bytes
> copied to the output string.  It doesn't copy a NUL.  We pass
> HV_KVP_EXCHANGE_MAX_VALUE_SIZE bytes as the limit.  So it fills up
> the buffer with non-null characters and we have an off-by-one.
> 
Dan,
I am sorry for not being as precise as I should be:
utf16s_to_utf8s() takes two length parameters - the length of the utf16 string
that is to be converted and the second the length of the utf8 output string.
The windows host manipulates all string in utf16 encoding and the string we get
from the host is guaranteed to be less than or equal to MAX value that we have
including the terminating character. In my code, I simply pass the length of the 
utf16 string as received from the host.

The parameter that I am currently passing MAX length value is the "maxout" 
parameter of the utf16s_utf8s() function. This by definition is the size of the
output buffer and in this case it happens to be MAX characters big.

> > I add one to take into account the string
> > terminator character for further processing. As I said before the
> > MAX value takes into account the terminating character for all the
> > strings handled.
> 
> So you're saying that since we control the input string, we'll never
> hit the max?  Still, why not pass HV_KVP_EXCHANGE_MAX_VALUE_SIZE - 1
> to leave room for the NUL just for correctness?  We'd still add one
> to the return value but we wouldn't go over the size of the buffer.

As I described earlier, with the  host side guarantee that the string we get from
the host is always guaranteed to be less than or equal to MAX length
(including the terminating character), there is no question of going over the
size of the output buffer which is sized based on the host specification.

Regards,

K. Y


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/