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. > 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. Again, I don't really know how utf16s_to_utf8s() works so I might have misunderstood. regards, dan carpenter
Attachment:
signature.asc
Description: Digital signature
_______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
