Re: [PATCH 1/2] fs: configfs: make qw_sign attribute symmetric

From: Krzysztof Opasiak
Date: Wed Apr 19 2017 - 04:54:02 EST




On 04/15/2017 03:35 AM, Stefan Agner wrote:
Currently qw_sign requires UTF-8 character to set, but returns UTF-16
when read. This isn't obvious when simply using cat since the null
characters are not visible, but hexdump unveils the true string:

# echo MSFT100 > os_desc/qw_sign
# hexdump -C os_desc/qw_sign
00000000 4d 00 53 00 46 00 54 00 31 00 30 00 30 00 |M.S.F.T.1.0.0.|

Make qw_sign symmetric by returning an UTF-8 string too. Also follow
common convention and add a new line at the end.

Signed-off-by: Stefan Agner <stefan@xxxxxxxx>
---
Resend as discussed here:
https://patchwork.kernel.org/patch/9548869/

Sorry, a bit later than we discussed... Hope still not too late?

--
Stefan

drivers/usb/gadget/configfs.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c
index cbff3b02840d..863ca4ded1be 100644
--- a/drivers/usb/gadget/configfs.c
+++ b/drivers/usb/gadget/configfs.c
@@ -787,9 +787,13 @@ static ssize_t os_desc_b_vendor_code_store(struct config_item *item,
static ssize_t os_desc_qw_sign_show(struct config_item *item, char *page)
{
struct gadget_info *gi = os_desc_item_to_gadget_info(item);
+ int res;

- memcpy(page, gi->qw_sign, OS_STRING_QW_SIGN_LEN);
- return OS_STRING_QW_SIGN_LEN;
+ res = utf16s_to_utf8s((wchar_t *) gi->qw_sign, OS_STRING_QW_SIGN_LEN,
+ UTF16_LITTLE_ENDIAN, page, PAGE_SIZE - 1);
+ page[res++] = '\n';
+
+ return res;
}

static ssize_t os_desc_qw_sign_store(struct config_item *item, const char *page,


Code itself looks good to me and from libusbgx perspective it's also fine to add this new line as we can just drop it like we do with other newlines in case of gadget/config strings.

Reviewed-by: Krzysztof Opasiak <k.opasiak@xxxxxxxxxxx>

Best regards,
--
Krzysztof Opasiak
Samsung R&D Institute Poland
Samsung Electronics