Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1650323 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 1/2] fs: configfs: make qw_sign attribute symmetric Stefan Agner <stefan@agner.ch> - 2017-05-25 10:40 +0200
    Re: [PATCH 1/2] fs: configfs: make qw_sign attribute symmetric Felipe Balbi <balbi@kernel.org> - 2017-06-02 10:30 +0200

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

Hi Felipe,

On 2017-04-19 01:53, Krzysztof Opasiak wrote:
> 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@agner.ch>
>> ---
>> 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@samsung.com>
> 

Any chance we get this in this merge window?

This still applies fine on v4.12-rc2.

--
Stefan

[toc] | [next] | [standalone]

#1656001

[Multipart message — attachments visible in raw view] — view raw

Hi,

Stefan Agner <stefan@agner.ch> writes:
> Hi Felipe,
>
> On 2017-04-19 01:53, Krzysztof Opasiak wrote:
>> 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@agner.ch>
>>> ---
>>> 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@samsung.com>
>> 
>
> Any chance we get this in this merge window?
>
> This still applies fine on v4.12-rc2.

I wouldn't consider this a fix. I'll get this into v4.13 merge window.

-- 
balbi

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web