RE: [NDCTL PATCH] ndctl: cxl: Remove dependency for attributes derived from IDENTIFY command

[Wonjae Lee]

On Fri, Feb 09, 2024 at 02:39:17PM -0700, Dave Jiang wrote:
> A memdev may optionally not host a mailbox and therefore not able to execute
> the IDENTIFY command. Currently the kernel emits empty strings for some of
> the attributes instead of making them invisible in order to keep backward
> compatibility for CXL CLI. Remove dependency of CXL CLI on the existance of
> these attributes and only expose them if they exist. Without the dependency
> the kernel will be able to make the non-existant attributes invisible.
>
> Link: https://lore.kernel.org/all/20230606121534.00003...@huawei.com/
> Suggested-by: Dan Williams <dan.j.willi...@intel.com>
> Signed-off-by: Dave Jiang <dave.ji...@intel.com>
> ---
>  cxl/lib/libcxl.c | 48 ++++++++++++++++++++++++++----------------------
>  cxl/memdev.c     | 15 ++++++++++-----
>  2 files changed, 36 insertions(+), 27 deletions(-)
>

[snip]

> diff --git a/cxl/memdev.c b/cxl/memdev.c
> index 81f07991da06..feab7ea76e78 100644
> --- a/cxl/memdev.c
> +++ b/cxl/memdev.c
> @@ -473,10 +473,13 @@ static int action_zero(struct cxl_memdev *memdev, 
> struct action_context *actx)
>   size_t size;
>   int rc;
>
> - if (param.len)
> + if (param.len) {
>       size = param.len;
> - else
> + } else {
>       size = cxl_memdev_get_label_size(memdev);
> +     if (size == ULLONG_MAX)
> +         return -EINVAL;
> + }

Hello,

Doesn't action_write() also need to check the return value of
cxl_memdev_get_label_size() like below?

diff --git a/cxl/memdev.c b/cxl/memdev.c
index feab7ea..de46edc 100644
--- a/cxl/memdev.c
+++ b/cxl/memdev.c
@@ -511,6 +511,8 @@ static int action_write(struct cxl_memdev *memdev, struct 
action_context *actx)

        if (!size) {
                size_t label_size = cxl_memdev_get_label_size(memdev);
+               if (label_size == ULLONG_MAX)
+                       return -EINVAL;

                fseek(actx->f_in, 0L, SEEK_END);
                size = ftell(actx->f_in);

Thanks,
Wonjae