Hi Raslan,
I sent a patch earlier regarding this but we will use yours.
Two small nits below.
On Thu, Aug 03, 2017 at 04:09:58PM +0300, rasl...@mellanox.com wrote:
> From: Raslan Darawsheh <rasl...@mellanox.com>
>
> When there is no prefered device, failsafe will always
> try to scan for prefered device. And if there is no device
> found with the exec option, popen() will get an empty output.
> In this case, it was forgotten to close the file descriptor.o
> it is fixed by closing the file descriptor even if the output is emtpy.
>
Good job on finding the issue and fixing it.
It has been assigned a coverity ID:
Coverity issue: 158633
> Fixes: a0194d82 ("net/failsafe: add flexible device definition")
>
The fixline should be 12-char long I think:
Fixes: a0194d828100 ("net/failsafe: add flexible device definition")
> Signed-off-by: Raslan Darawsheh <rasl...@mellanox.com>
Otherwise:
Acked-by: Gaetan Rivet <gaetan.ri...@6wind.com>
> ---
> drivers/net/failsafe/failsafe_args.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/failsafe/failsafe_args.c
> b/drivers/net/failsafe/failsafe_args.c
> index 932e371..3f92a77 100644
> --- a/drivers/net/failsafe/failsafe_args.c
> +++ b/drivers/net/failsafe/failsafe_args.c
> @@ -115,7 +115,7 @@ fs_execute_cmd(struct sub_device *sdev, char *cmdline)
> char output[DEVARGS_MAXLEN + 1];
> size_t len;
> int old_err;
> - int ret;
> + int ret, pclose_ret;
>
> RTE_ASSERT(cmdline != NULL || sdev->cmdline != NULL);
> if (sdev->cmdline == NULL) {
> @@ -145,7 +145,8 @@ fs_execute_cmd(struct sub_device *sdev, char *cmdline)
> /* We only read one line */
> if (fgets(output, sizeof(output) - 1, fp) == NULL) {
> DEBUG("Could not read command output");
> - return -ENODEV;
> + ret = -ENODEV;
> + goto ret_pclose;
> }
> fs_sanitize_cmdline(output);
> ret = fs_parse_device(sdev, output);
> @@ -154,12 +155,12 @@ fs_execute_cmd(struct sub_device *sdev, char *cmdline)
> goto ret_pclose;
> }
> ret_pclose:
> - ret = pclose(fp);
> - if (ret) {
> - ret = errno;
> + pclose_ret = pclose(fp);
> + if (pclose_ret) {
> + pclose_ret = errno;
> ERROR("pclose: %s", strerror(errno));
> errno = old_err;
> - return ret;
> + return pclose_ret;
> }
> return ret;
> }
> --
> 2.7.4
>
--
Gaëtan Rivet
6WIND
