Re: [PATCH v3] cxl/acpi: Cleanup __cxl_parse_cfmws()

[Alison Schofield <alison.schofield@intel.com>]

On Fri, Apr 05, 2024 at 03:05:50PM -0700, Dan Williams wrote:
> As a follow on to the recent rework of __cxl_parse_cfmws() to always
> return errors [1], use cleanup.h helpers to remove goto and other cleanups
> now that logging is moved to the cxl_parse_cfmws() wrapper.
> 
> This ends up adding more code than it deletes, but __cxl_parse_cfmws()
> itself does get smaller. The takeaway from the cond_no_free_ptr()
> discussion [2] was to not add new macros to handle the cases where
> no_free_ptr() is awkward, instead rework the code to have helpers and
> clearer delineation of responsibility.
> 
> Now one might say that  __free(del_cxl_resource) is excessive given it
> is immediately registered with add_or_reset_cxl_resource(). The
> rationale for keeping it is that it forces use of "no_free_ptr()" on the
> argument passed to add_or_reset_cxl_resource(). That in turn makes it
> clear that @res is NULL for the rest of the function which is part of
> the point of the cleanup helpers, to turn subtle use after free errors
> [3] into loud NULL pointer de-references.
> 
> Link: http://lore.kernel.org/r/170820177238.631006.1012639681618409284.stgit@dwillia2-xfh.jf.intel.com [1]
> Link: http://lore.kernel.org/r/CAHk-=whBVhnh=KSeBBRet=E7qJAwnPR_aj5em187Q3FiD+LXnA@mail.gmail.com [2]
> Link: http://lore.kernel.org/r/20230714093146.2253438-1-leitao@debian.org [3]
> Reported-by: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
> Closes: http://lore.kernel.org/r/20240219124041.00002bda@Huawei.com
> Reviewed-by: Alison Schofield <alison.schofield@intel.com>
> Reviewed-by: Vishal Verma <vishal.l.verma@intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>


Doubling down ;)

Reviewed-by: Alison Schofield <alison.schofield@intel.com>

> ---
> Changes since v2:
> - Drop kvasprintf() usage in alloc_cxl_resource (Alison)
> 
>  drivers/cxl/acpi.c |   93 +++++++++++++++++++++++++++++-----------------------
>  drivers/cxl/cxl.h  |    5 +++
>  2 files changed, 56 insertions(+), 42 deletions(-)

snip