> -----Original Message----- > From: Dan Carpenter [mailto:dan.carpenter@xxxxxxxxxx] > Sent: Sunday, March 11, 2012 6:43 AM > 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 Sat, Mar 10, 2012 at 03:32:09PM -0800, K. Y. Srinivasan wrote: > > + switch (message->kvp_hdr.operation) { > > + case KVP_OP_SET: > > + switch (in_msg->body.kvp_set.data.value_type) { > > + case REG_SZ: > > + /* > > + * The value is a string - utf16 encoding. > > + */ > > + message->body.kvp_set.data.value_size = > > + utf16s_to_utf8s( > > + (wchar_t *) > > + in_msg->body.kvp_set.data.value, > > + in_msg->body.kvp_set.data.value_size, > > + UTF16_LITTLE_ENDIAN, > > + message->body.kvp_set.data.value, > > + HV_KVP_EXCHANGE_MAX_VALUE_SIZE) + 1; > > + break; > > + > > This block of unreadable text is so nasty. > > You could return directly if the msg = kmalloc() fails and pull > everything in one indent level. It's normally more readable to > handle errors as soon as possible anyway. True. > > 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". Dan, I will see if there are other comments on these patches and will accommodate your suggestion then. If there are no other comments, would you mind if I addressed your comments here in a separate patch. Regards, K. Y > > regards, > dan carpenter _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
