From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754501AbdBISFF (ORCPT ); Thu, 9 Feb 2017 13:05:05 -0500 Received: from mail.kmu-office.ch ([178.209.48.109]:49548 "EHLO mail.kmu-office.ch" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753276AbdBISEy (ORCPT ); Thu, 9 Feb 2017 13:04:54 -0500 MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Date: Thu, 09 Feb 2017 10:04:43 -0800 From: Stefan Agner To: Greg KH Cc: balbi@kernel.org, andrzej.p@samsung.com, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 1/2] fs: configfs: make qw_sign attribute symmetric In-Reply-To: References: <20170201021917.27398-1-stefan@agner.ch> <20170201080616.GA17968@kroah.com> Message-ID: <32e2fa73fdbd8b52abe442365fd1c514@agner.ch> User-Agent: Roundcube Webmail/1.1.3 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2017-02-01 08:59, Stefan Agner wrote: > On 2017-02-01 00:06, Greg KH wrote: >> On Tue, Jan 31, 2017 at 06:19:16PM -0800, 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. >> >> Doesn't USB require that strings be in UTF-16? So why have the kernel >> convert them? > > That is a discussion we should have had when the write side of this has > been added: > > static ssize_t os_desc_qw_sign_store(struct config_item *item, const > char *page, > size_t len) > { > struct gadget_info *gi = os_desc_item_to_gadget_info(item); > int res, l; > > l = min((int)len, OS_STRING_QW_SIGN_LEN >> 1); > if (page[l - 1] == '\n') > --l; > > mutex_lock(&gi->lock); > res = utf8s_to_utf16s(page, l, > UTF16_LITTLE_ENDIAN, (wchar_t *) gi->qw_sign, > OS_STRING_QW_SIGN_LEN); > if (res > 0) > res = len; > mutex_unlock(&gi->lock); > > return res; > } > > > The store function is definitely already in use today, e.g. this script > used for ev3dev: > https://github.com/ev3dev/ev3-systemd/blob/ev3dev-jessie/scripts/ev3-usb.sh > > Changing it to UTF-16 would break that script... So changing the store > part is the lesser of two evils. > > Regarding new line: Just following what other attributes are doing by > using the GS_STRINGS_R macro. Any comment on this? In my opinion especially this first patch really fixes a bug and should get applied... I can remove the newline if preferred. -- Stefan