On Fri, 2017-05-05 at 10:06 +0300, Amir Goldstein wrote:
On Fri, May 5, 2017 at 9:20 AM, Dan Williams <dan.j.williams@intel.com
wrote: On Thu, May 4, 2017 at 2:21 AM, Andy Shevchenko andriy.shevchenko@linux.intel.com wrote:
for (i = 0; i < NFIT_UUID_MAX; i++) - if (memcmp(to_nfit_uuid(i), spa->range_guid, 16) == 0) + if (!uuid_le_cmp_pp(to_nfit_uuid(i), (uuid_le *)spa->range_guid))
What is _cmp_pp? Why not _compare?
Dan, it's a typo. In this patch it should be just ..._cmp(), which is already a part of API.
I second that.
Andy,
Amir, just to be clear. This patch can be applied without any addons to an existing API. Above is just a typo due to rebase in my tree. I will replace it to just uuid_le_cmp().
I much rather that you sort out uuid helpers in a way that will satisfy the filesystem needs (just provide the helpers don't need to convert filesystems code).
The only reason I took a swing at hoisting the xfs uuid helpers is because it didn't seem like your proposal was going to be posted soon or wasn't going to satisfy the filesystems use case.
My opinion now, is that your suggestion is probably much closer to the real deal than mine.
IMO, you should acknowledge that the common use case for filesystems is to handle an opaque char[16] which most likely holds a uuid_be and you should provide 'neutral' helpers to satisfy this use case.
The simplest would be to typedef uuid_t to struct uuid_be and to name 'neutral' helpers' uuid_cmp/uuid_copy(uuid_t *, uuid_t *), similar to my proposal.
I think with this semantic change, our proposals can reach common grounds and satisfy a wider group of users (i.e. filesystem developers).
Christoph also suggested a similar treatment to typedef guid_t to struct uuid_le. I don't know the use cases enough to comment on that.
We may go this way. But I wouldn't prevent current users of uuid_le to continue using it without conversion (it may be done case by case after we settle an API)
So, summarize what Christoph said it will look like
typedef uuid_be uuid_t; typedef uuid_le guid_t
uuid_cmp() / uuid_copy() / uuid_to_bin() / etc guid_cmp() / guid_copy() / guid_to_bin() / etc
Correct? Christoph?