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

[Dan Williams <dan.j.williams@intel.com>]

On Wed, Aug 11, 2021 at 10:27 AM Jonathan Cameron
<Jonathan.Cameron@huawei.com> wrote:
>
> 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.

Sure, easy to do.

>
> 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;

Ok.

>
> more easily as a clear "error" return.
>
> > +     return memcpy(nd_label->name, name, NSLABEL_NAME_LEN);
> > +}
> > +
> ...