From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andy Shevchenko Subject: Re: [PATCH v1 06/10] device property: switch to use UUID API Date: Thu, 07 Apr 2016 19:41:26 +0300 Message-ID: <1460047286.6620.26.camel@linux.intel.com> References: <1455711448-124103-1-git-send-email-andriy.shevchenko@linux.intel.com> <1455711448-124103-7-git-send-email-andriy.shevchenko@linux.intel.com> <7544228.v4QPX4F7J7@vostro.rjw.lan> <1456495897.13244.144.camel@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: In-Reply-To: <1456495897.13244.144.camel@linux.intel.com> Sender: linux-acpi-owner@vger.kernel.org To: "Rafael J. Wysocki" , Huang Ying Cc: Theodore Ts'o , Arnd Bergmann , Greg Kroah-Hartman , Jarkko Sakkinen , Jani Nikula , David Airlie , Benjamin Tissoires , Bjorn Helgaas , Mathias Nyman , Matt Fleming , Lv Zheng , Mark Brown , Zhang Rui , Mika Westerberg , Andrew Morton , Rasmus Villemoes , linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-efi@vger.kernel.org, linux-api@vger.kernel.org, linux-nvdimm@lists.01.org List-Id: linux-api@vger.kernel.org On Fri, 2016-02-26 at 16:11 +0200, Andy Shevchenko wrote: > On Thu, 2016-02-18 at 01:03 +0100, Rafael J. Wysocki wrote: > >=20 > > On Wednesday, February 17, 2016 02:17:24 PM Andy Shevchenko wrote: > > >=20 > > > Switch to use a generic UUID API instead of custom approach. It > > > allows to > > > define UUIDs, compare them, and validate. > [] >=20 Summon initial author of the UUID library. Summary: the API of comparison functions is rather strange. What the point to not take pointers directly? (Moreover I hope compiler too clever not to make a copy of constant arguments there) I could only imagine the case you are trying to avoid temporary variables for constants like NULL_UUID. Issue with this is the ugliness in the users of that, in particularly present in ACPI (drivers/acpi/apei/ghes.c). I would like to have more clear interface for that. Perhaps we may add something like cmp_p(pointer, non-pointer); cmp_pp(pointer, pointer); to not break existing API for now. It would be useful for many cases in the kernel. > >=20 > > >=20 > > > +static const uuid_le ads_uuid =3D > > > + UUID_LE(0xdbb8e3e6, 0x5886, 0x4ba6, > > > + 0x87, 0x95, 0x13, 0x19, 0xf5, 0x2a, 0x96, 0x6b); > > > =C2=A0 > > > =C2=A0static bool acpi_enumerate_nondev_subnodes(acpi_handle scop= e, > > > =C2=A0 =C2=A0=C2=A0=C2=A0const union > > > acpi_object > > > *desc, > > > @@ -138,7 +136,7 @@ static bool > > > acpi_enumerate_nondev_subnodes(acpi_handle scope, > > > =C2=A0 =C2=A0=C2=A0=C2=A0=C2=A0|| links->type !=3D ACPI_TYPE_PAC= KAGE) > > > =C2=A0 break; > > > =C2=A0 > > > - if (memcmp(uuid->buffer.pointer, ads_uuid, > > > sizeof(ads_uuid))) > > > + if (uuid_le_cmp(*(uuid_le *)uuid->buffer.pointer, > > > ads_uuid)) > > Maybe it's too late, but I don't quite understand the pointer > > manipulations here. > >=20 > > I can see why you need a type conversion (although it looks ugly), > > but why do you > > need to dereference it too? > The function takes that kind of type on input. The other variants are > not compiled. > Perhaps we better change uuid_{lb}e_cmp() first to take normal > pointers, though I think the initial idea was to get type checking at > compile time. >=20 --=20 Andy Shevchenko Intel Finland Oy -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" i= n the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by ml01.01.org (Postfix) with ESMTP id 867B21A1F40 for ; Thu, 7 Apr 2016 09:40:33 -0700 (PDT) Message-ID: <1460047286.6620.26.camel@linux.intel.com> Subject: Re: [PATCH v1 06/10] device property: switch to use UUID API From: Andy Shevchenko Date: Thu, 07 Apr 2016 19:41:26 +0300 In-Reply-To: <1456495897.13244.144.camel@linux.intel.com> References: <1455711448-124103-1-git-send-email-andriy.shevchenko@linux.intel.com> <1455711448-124103-7-git-send-email-andriy.shevchenko@linux.intel.com> <7544228.v4QPX4F7J7@vostro.rjw.lan> <1456495897.13244.144.camel@linux.intel.com> Mime-Version: 1.0 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Errors-To: linux-nvdimm-bounces@lists.01.org Sender: "Linux-nvdimm" To: "Rafael J. Wysocki" , Huang Ying Cc: linux-efi@vger.kernel.org, David Airlie , Rasmus Villemoes , dri-devel@lists.freedesktop.org, Benjamin Tissoires , Lv Zheng , Matt Fleming , Arnd Bergmann , linux-nvdimm@lists.01.org, linux-acpi@vger.kernel.org, Zhang Rui , Mathias Nyman , Jani Nikula , Mark Brown , Jarkko Sakkinen , Bjorn Helgaas , Mika Westerberg , Theodore Ts'o , Greg Kroah-Hartman , linux-kernel@vger.kernel.org, linux-api@vger.kernel.org, Andrew Morton List-ID: T24gRnJpLCAyMDE2LTAyLTI2IGF0IDE2OjExICswMjAwLCBBbmR5IFNoZXZjaGVua28gd3JvdGU6 Cj4gT24gVGh1LCAyMDE2LTAyLTE4IGF0IDAxOjAzICswMTAwLCBSYWZhZWwgSi4gV3lzb2NraSB3 cm90ZToKPiA+IAo+ID4gT24gV2VkbmVzZGF5LCBGZWJydWFyeSAxNywgMjAxNiAwMjoxNzoyNCBQ TSBBbmR5IFNoZXZjaGVua28gd3JvdGU6Cj4gPiA+IAo+ID4gPiBTd2l0Y2ggdG8gdXNlIGEgZ2Vu ZXJpYyBVVUlEIEFQSSBpbnN0ZWFkIG9mIGN1c3RvbSBhcHByb2FjaC4gSXQKPiA+ID4gYWxsb3dz IHRvCj4gPiA+IGRlZmluZSBVVUlEcywgY29tcGFyZSB0aGVtLCBhbmQgdmFsaWRhdGUuCj4gW10K PiAKClN1bW1vbiBpbml0aWFsIGF1dGhvciBvZiB0aGUgVVVJRCBsaWJyYXJ5LgoKU3VtbWFyeTog dGhlIEFQSSBvZiBjb21wYXJpc29uIGZ1bmN0aW9ucyBpcyByYXRoZXIgc3RyYW5nZS4gV2hhdCB0 aGUKcG9pbnQgdG8gbm90IHRha2UgcG9pbnRlcnMgZGlyZWN0bHk/IChNb3Jlb3ZlciBJIGhvcGUg Y29tcGlsZXIgdG9vCmNsZXZlciBub3QgdG8gbWFrZSBhIGNvcHkgb2YgY29uc3RhbnQgYXJndW1l bnRzIHRoZXJlKQoKSSBjb3VsZCBvbmx5IGltYWdpbmUgdGhlIGNhc2UgeW91IGFyZSB0cnlpbmcg dG8gYXZvaWQgdGVtcG9yYXJ5CnZhcmlhYmxlcyBmb3IgY29uc3RhbnRzIGxpa2UgTlVMTF9VVUlE LgoKSXNzdWUgd2l0aCB0aGlzIGlzIHRoZSB1Z2xpbmVzcyBpbiB0aGUgdXNlcnMgb2YgdGhhdCwg aW4gcGFydGljdWxhcmx5CnByZXNlbnQgaW4gQUNQSSAoZHJpdmVycy9hY3BpL2FwZWkvZ2hlcy5j KS4KCkkgd291bGQgbGlrZSB0byBoYXZlIG1vcmUgY2xlYXIgaW50ZXJmYWNlIGZvciB0aGF0LiBQ ZXJoYXBzIHdlIG1heSBhZGQKc29tZXRoaW5nIGxpa2UKCmNtcF9wKHBvaW50ZXIsIG5vbi1wb2lu dGVyKTsKY21wX3BwKHBvaW50ZXIsIHBvaW50ZXIpOwoKdG8gbm90IGJyZWFrIGV4aXN0aW5nIEFQ SSBmb3Igbm93LgoKSXQgd291bGQgYmUgdXNlZnVsIGZvciBtYW55IGNhc2VzIGluIHRoZSBrZXJu ZWwuCgo+ID4gCj4gPiA+IAo+ID4gPiArc3RhdGljIGNvbnN0IHV1aWRfbGUgYWRzX3V1aWQgPQo+ ID4gPiArCVVVSURfTEUoMHhkYmI4ZTNlNiwgMHg1ODg2LCAweDRiYTYsCj4gPiA+ICsJCTB4ODcs IDB4OTUsIDB4MTMsIDB4MTksIDB4ZjUsIDB4MmEsIDB4OTYsIDB4NmIpOwo+ID4gPiDCoAo+ID4g PiDCoHN0YXRpYyBib29sIGFjcGlfZW51bWVyYXRlX25vbmRldl9zdWJub2RlcyhhY3BpX2hhbmRs ZSBzY29wZSwKPiA+ID4gwqAJCQkJCcKgwqDCoGNvbnN0IHVuaW9uCj4gPiA+IGFjcGlfb2JqZWN0 Cj4gPiA+ICpkZXNjLAo+ID4gPiBAQCAtMTM4LDcgKzEzNiw3IEBAIHN0YXRpYyBib29sCj4gPiA+ IGFjcGlfZW51bWVyYXRlX25vbmRldl9zdWJub2RlcyhhY3BpX2hhbmRsZSBzY29wZSwKPiA+ID4g wqAJCcKgwqDCoMKgfHwgbGlua3MtPnR5cGUgIT0gQUNQSV9UWVBFX1BBQ0tBR0UpCj4gPiA+IMKg CQkJYnJlYWs7Cj4gPiA+IMKgCj4gPiA+IC0JCWlmIChtZW1jbXAodXVpZC0+YnVmZmVyLnBvaW50 ZXIsIGFkc191dWlkLAo+ID4gPiBzaXplb2YoYWRzX3V1aWQpKSkKPiA+ID4gKwkJaWYgKHV1aWRf bGVfY21wKCoodXVpZF9sZSAqKXV1aWQtPmJ1ZmZlci5wb2ludGVyLAo+ID4gPiBhZHNfdXVpZCkp Cj4gPiBNYXliZSBpdCdzIHRvbyBsYXRlLCBidXQgSSBkb24ndCBxdWl0ZSB1bmRlcnN0YW5kIHRo ZSBwb2ludGVyCj4gPiBtYW5pcHVsYXRpb25zIGhlcmUuCj4gPiAKPiA+IEkgY2FuIHNlZSB3aHkg eW91IG5lZWQgYSB0eXBlIGNvbnZlcnNpb24gKGFsdGhvdWdoIGl0IGxvb2tzIHVnbHkpLAo+ID4g YnV0IHdoeSBkbyB5b3UKPiA+IG5lZWQgdG8gZGVyZWZlcmVuY2UgaXQgdG9vPwo+IFRoZSBmdW5j dGlvbiB0YWtlcyB0aGF0IGtpbmQgb2YgdHlwZSBvbiBpbnB1dC4gVGhlIG90aGVyIHZhcmlhbnRz IGFyZQo+IG5vdCBjb21waWxlZC4KPiBQZXJoYXBzIHdlIGJldHRlciBjaGFuZ2UgdXVpZF97bGJ9 ZV9jbXAoKSBmaXJzdCB0byB0YWtlIG5vcm1hbAo+IHBvaW50ZXJzLCB0aG91Z2ggSSB0aGluayB0 aGUgaW5pdGlhbCBpZGVhIHdhcyB0byBnZXQgdHlwZSBjaGVja2luZyBhdAo+IGNvbXBpbGUgdGlt ZS4KPiAKCi0tIApBbmR5IFNoZXZjaGVua28gPGFuZHJpeS5zaGV2Y2hlbmtvQGxpbnV4LmludGVs LmNvbT4KSW50ZWwgRmlubGFuZCBPeQoKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX18KTGludXgtbnZkaW1tIG1haWxpbmcgbGlzdApMaW51eC1udmRpbW1AbGlz dHMuMDEub3JnCmh0dHBzOi8vbGlzdHMuMDEub3JnL21haWxtYW4vbGlzdGluZm8vbGludXgtbnZk aW1tCg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756912AbcDGQkh (ORCPT ); Thu, 7 Apr 2016 12:40:37 -0400 Received: from mga02.intel.com ([134.134.136.20]:63010 "EHLO mga02.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756453AbcDGQkf (ORCPT ); Thu, 7 Apr 2016 12:40:35 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.24,449,1455004800"; d="scan'208";a="927663683" Message-ID: <1460047286.6620.26.camel@linux.intel.com> Subject: Re: [PATCH v1 06/10] device property: switch to use UUID API From: Andy Shevchenko To: "Rafael J. Wysocki" , Huang Ying Cc: "Theodore Ts'o" , Arnd Bergmann , Greg Kroah-Hartman , Jarkko Sakkinen , Jani Nikula , David Airlie , Benjamin Tissoires , Bjorn Helgaas , Mathias Nyman , Matt Fleming , Lv Zheng , Mark Brown , Zhang Rui , Mika Westerberg , Andrew Morton , Rasmus Villemoes , linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-efi@vger.kernel.org, linux-api@vger.kernel.org, linux-nvdimm@ml01.01.org Date: Thu, 07 Apr 2016 19:41:26 +0300 In-Reply-To: <1456495897.13244.144.camel@linux.intel.com> References: <1455711448-124103-1-git-send-email-andriy.shevchenko@linux.intel.com> <1455711448-124103-7-git-send-email-andriy.shevchenko@linux.intel.com> <7544228.v4QPX4F7J7@vostro.rjw.lan> <1456495897.13244.144.camel@linux.intel.com> Organization: Intel Finland Oy Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.18.5.1-1 Mime-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 2016-02-26 at 16:11 +0200, Andy Shevchenko wrote: > On Thu, 2016-02-18 at 01:03 +0100, Rafael J. Wysocki wrote: > > > > On Wednesday, February 17, 2016 02:17:24 PM Andy Shevchenko wrote: > > > > > > Switch to use a generic UUID API instead of custom approach. It > > > allows to > > > define UUIDs, compare them, and validate. > [] > Summon initial author of the UUID library. Summary: the API of comparison functions is rather strange. What the point to not take pointers directly? (Moreover I hope compiler too clever not to make a copy of constant arguments there) I could only imagine the case you are trying to avoid temporary variables for constants like NULL_UUID. Issue with this is the ugliness in the users of that, in particularly present in ACPI (drivers/acpi/apei/ghes.c). I would like to have more clear interface for that. Perhaps we may add something like cmp_p(pointer, non-pointer); cmp_pp(pointer, pointer); to not break existing API for now. It would be useful for many cases in the kernel. > > > > > > > > +static const uuid_le ads_uuid = > > > + UUID_LE(0xdbb8e3e6, 0x5886, 0x4ba6, > > > + 0x87, 0x95, 0x13, 0x19, 0xf5, 0x2a, 0x96, 0x6b); > > >   > > >  static bool acpi_enumerate_nondev_subnodes(acpi_handle scope, > > >      const union > > > acpi_object > > > *desc, > > > @@ -138,7 +136,7 @@ static bool > > > acpi_enumerate_nondev_subnodes(acpi_handle scope, > > >       || links->type != ACPI_TYPE_PACKAGE) > > >   break; > > >   > > > - if (memcmp(uuid->buffer.pointer, ads_uuid, > > > sizeof(ads_uuid))) > > > + if (uuid_le_cmp(*(uuid_le *)uuid->buffer.pointer, > > > ads_uuid)) > > Maybe it's too late, but I don't quite understand the pointer > > manipulations here. > > > > I can see why you need a type conversion (although it looks ugly), > > but why do you > > need to dereference it too? > The function takes that kind of type on input. The other variants are > not compiled. > Perhaps we better change uuid_{lb}e_cmp() first to take normal > pointers, though I think the initial idea was to get type checking at > compile time. > -- Andy Shevchenko Intel Finland Oy