On Tue, Jun 15, 2021 at 08:39:20PM +0800, Zhiqiang Liu wrote:
> 
> In do_xaction_namespace(), ri_ctx.f_out should be closed after
> being opened.
> 

Hi Zhiqiang,

The commit message and commit log need to be swapped.

Something like:

Commit message says what the patch does:
[ndctl PATCH 2/2] namespace: Close fd before return in do_xaction_namespace()

Commit log says why it needs to be done:
This prevents a potential file descriptor leak in do_xaction_namespace()

And, same as in Patch 1 - mention it was found by Coverity.

Alison

> Signed-off-by: Zhiqiang Liu <[email protected]>
> ---
>  ndctl/namespace.c | 17 +++++++++--------
>  1 file changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/ndctl/namespace.c b/ndctl/namespace.c
> index 21089d7..55364ac 100644
> --- a/ndctl/namespace.c
> +++ b/ndctl/namespace.c
> @@ -2141,7 +2141,7 @@ static int do_xaction_namespace(const char *namespace,
>                               util_display_json_array(ri_ctx.f_out, 
> ri_ctx.jblocks, 0);
>                       if (rc >= 0)
>                               (*processed)++;
> -                     return rc;
> +                     goto out;
>               }
>       }
> 
> @@ -2152,11 +2152,11 @@ static int do_xaction_namespace(const char *namespace,
>               rc = file_write_infoblock(param.outfile);
>               if (rc >= 0)
>                       (*processed)++;
> -             return rc;
> +             goto out;
>       }
> 
>       if (!namespace && action != ACTION_CREATE)
> -             return rc;
> +             goto out;
> 
>       if (verbose)
>               ndctl_set_log_priority(ctx, LOG_DEBUG);
> @@ -2212,7 +2212,7 @@ static int do_xaction_namespace(const char *namespace,
>                                               saved_rc = rc;
>                                               continue;
>                               }
> -                             return rc;
> +                             goto out;
>                       }
>                       ndctl_namespace_foreach_safe(region, ndns, _n) {
>                               ndns_name = ndctl_namespace_get_devname(ndns);
> @@ -2259,7 +2259,7 @@ static int do_xaction_namespace(const char *namespace,
>                                       rc = namespace_reconfig(region, ndns);
>                                       if (rc == 0)
>                                               *processed = 1;
> -                                     return rc;
> +                                     goto out;
>                               case ACTION_READ_INFOBLOCK:
>                                       rc = namespace_rw_infoblock(ndns, 
> &ri_ctx, READ);
>                                       if (rc == 0)
> @@ -2281,9 +2281,6 @@ static int do_xaction_namespace(const char *namespace,
>       if (ri_ctx.jblocks)
>               util_display_json_array(ri_ctx.f_out, ri_ctx.jblocks, 0);
> 
> -     if (ri_ctx.f_out && ri_ctx.f_out != stdout)
> -             fclose(ri_ctx.f_out);
> -
>       if (action == ACTION_CREATE && rc == -EAGAIN) {
>               /*
>                * Namespace creation searched through all candidate
> @@ -2301,6 +2298,10 @@ static int do_xaction_namespace(const char *namespace,
>       if (saved_rc)
>               rc = saved_rc;
> 
> +out:
> +     if (ri_ctx.f_out && ri_ctx.f_out != stdout)
> +             fclose(ri_ctx.f_out);
> +
>       return rc;
>  }
> 
> -- 
> 2.23.0
> 
> 
> 
> 
> .
> 
> 
> 

Reply via email to