Re: [PATCH 03/23] libnvdimm/labels: Introduce label setter helpers

[Jonathan Cameron <Jonathan.Cameron@Huawei.com>]

On Mon, 9 Aug 2021 15:28:03 -0700
Dan Williams <dan.j.williams@intel.com> wrote:

> In preparation for LIBNVDIMM to manage labels on CXL devices deploy
> helpers that abstract the label type from the implementation. The CXL
> label format is mostly similar to the EFI label format with concepts /
> fields added, like dynamic region creation and label type guids, and
> other concepts removed like BLK-mode and interleave-set-cookie ids.
> 
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

Hi Dan,

Only thing on this patch is whether it might be better to put get /set pairs
together rather than all the get functions, then all the set functions?

If looking at this code in future it would make it a little easier to quickly
see they are match pairs.

Your code though, so if you prefer it like this, I don't really care!

Fine either way with me.
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> ---
>  drivers/nvdimm/label.c          |   61 +++++++++++++++++------------------
>  drivers/nvdimm/namespace_devs.c |    2 +
>  drivers/nvdimm/nd.h             |   68 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 98 insertions(+), 33 deletions(-)
> 

...
  
> diff --git a/drivers/nvdimm/nd.h b/drivers/nvdimm/nd.h
> index b3feaf3699f7..416846fe7818 100644
> --- a/drivers/nvdimm/nd.h
> +++ b/drivers/nvdimm/nd.h
> @@ -47,6 +47,14 @@ static inline u8 *nsl_get_name(struct nvdimm_drvdata *ndd,
>  	return memcpy(name, nd_label->name, NSLABEL_NAME_LEN);
>  }
>  
> +static inline u8 *nsl_set_name(struct nvdimm_drvdata *ndd,
> +			       struct nd_namespace_label *nd_label, u8 *name)
> +{
> +	if (!name)
> +		return name;

Nitpick: Obviously same thing, but my eyes parse 
		return NULL;

more easily as a clear "error" return.

> +	return memcpy(nd_label->name, name, NSLABEL_NAME_LEN);
> +}
> +
...