From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752154AbdBMKrY (ORCPT ); Mon, 13 Feb 2017 05:47:24 -0500 Received: from mga05.intel.com ([192.55.52.43]:6153 "EHLO mga05.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751023AbdBMKrX (ORCPT ); Mon, 13 Feb 2017 05:47:23 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.35,156,1484035200"; d="asc'?scan'208";a="1125866766" From: Felipe Balbi To: Stefan Agner Cc: Greg KH , 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: <7ca2aab805111faca014b0faea0965df@agner.ch> References: <20170201021917.27398-1-stefan@agner.ch> <20170201080616.GA17968@kroah.com> <32e2fa73fdbd8b52abe442365fd1c514@agner.ch> <20170210111929.GA22347@kroah.com> <87h942qldi.fsf@linux.intel.com> <7ca2aab805111faca014b0faea0965df@agner.ch> Date: Mon, 13 Feb 2017 12:46:54 +0200 Message-ID: <8737fiqsf5.fsf@linux.intel.com> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha256; protocol="application/pgp-signature" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable Hi, Stefan Agner writes: >>> On Thu, Feb 09, 2017 at 10:04:43AM -0800, Stefan Agner wrote: >>>> 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 foll= ow >>>> >>> common convention and add a new line at the end. >>>> >> >>>> >> Doesn't USB require that strings be in UTF-16? So why have the ker= nel >>>> >> 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 =3D os_desc_item_to_gadget_info(item); >>>> > int res, l; >>>> > >>>> > l =3D min((int)len, OS_STRING_QW_SIGN_LEN >> 1); >>>> > if (page[l - 1] =3D=3D '\n') >>>> > --l; >>>> > >>>> > mutex_lock(&gi->lock); >>>> > res =3D utf8s_to_utf16s(page, l, >>>> > UTF16_LITTLE_ENDIAN, (wchar_t *) gi->qw_sign, >>>> > OS_STRING_QW_SIGN_LEN); >>>> > if (res > 0) >>>> > res =3D len; >>>> > mutex_unlock(&gi->lock); >>>> > >>>> > return res; >>>> > } >>>> > >>>> > >>>> > The store function is definitely already in use today, e.g. this scr= ipt >>>> > 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 sto= re >>>> > 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. >>> >>> It's up to Felipe, give him a chance to catch up on patches... >>=20 >> I really don't know what to do here :-) Either way have the potential of >> breaking userspace. Maybe returning UTF8 as in write is the lesser of >> two evils. > > So far libusbg and libusbgx do not have support for OS Descriptors. > > I came across the issue while writing support for libusbgx (not upstream > yet). And it is rather ugly to have to do it on the read side and not to > do on the write side. It would also extend the dependencies of the > library (I implemented a prototype using iconv). > > Also reading the old interface with UTF-8 does not crash the library, it > just return only the first character (since the second character is > \0...). Okay, then let's go with your original patch. Can you resend once -rc1 is out? =2D-=20 balbi --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEElLzh7wn96CXwjh2IzL64meEamQYFAlihjp8ACgkQzL64meEa mQbR2BAAwuV8wzhgFxCFh98cpegTlbzvMt4LtyQbD33YJXZf+4olQ//c+r3omy/K 8YT6pUk4W8zoIfYX+nM1PrhatP7ZEbz3g/oi5nz6Bgm++v/kfkNz3VAiL2MAhVZu meNIE6Idt7sIGbwhrLado60MsI+JTQ796FhNLDoJhgC1VOdX1OiyD1s6Z0ZejuZl ZrjXOeH261sb8jjnNVAmz1+Hnn+sFscAUgaqlpw3/5rksWIjTIs+vEArwZcHOmAn aYDAQn7IBudGjhkLOGQ7mQNyQxsuKle68KlrtlodwPquih1+FY9JoazCQ5Rb3ndl k5ld1ShMgJ6gHY8VsbODSL/QWlXOsJvkm4bH+HAZaMQcNFXk9KVSCEsmcncFjXbM /greYrXgLWKy4c6yfrzVkKGhGI2CfGsI8pWnL2pZAHefvPY9dFx7HwlsdfenmrAm Qy76PUdbYJHPR/PN7SgJIA+rvV5043d51t4Z/OnJFqy+FGqmob9Yqkvaw4ZLr0tu AyZ3lOERY1/CoKtgJb2KQD5e+OyFWVxWNnf5Ag+hbn56fMb+VNwTWwjw5uh3heS6 r+yGTHmf1PwP4l8s9OwbHFpCgWA9HoGqFb63ccOLrS3KM1HPrLpgbGcOuOBlfzFR +pzDDzand4Bxn6Ojn4CV6F2BUwnfEKqaSAKSA0Ssz6Nk5nKD5IA= =uM/F -----END PGP SIGNATURE----- --=-=-=--