diff mbox

[dpdk-dev] cmdline: fix warning for unused return value

Message ID 20170907130923.157924-1-bruce.richardson@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Thomas Monjalon
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Bruce Richardson Sept. 7, 2017, 1:09 p.m. UTC 
  When DPDK is compiled on Ubuntu with extra warnings turned on, there is a
warning about the return value from write() being unchecked. Rather than
having builds disable the warning, which may mask other cases we do care
about, we can add a dummy use of the return value in the code to silence it
in this instance.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/librte_cmdline/cmdline.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Olivier Matz Sept. 7, 2017, 1:49 p.m. UTC | #1 
On Thu, Sep 07, 2017 at 02:09:23PM +0100, Bruce Richardson wrote:
> When DPDK is compiled on Ubuntu with extra warnings turned on, there is a
> warning about the return value from write() being unchecked. Rather than
> having builds disable the warning, which may mask other cases we do care
> about, we can add a dummy use of the return value in the code to silence it
> in this instance.
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>

Acked-by: Olivier Matz <olivier.matz@6wind.com>
Stephen Hemminger Sept. 7, 2017, 5:50 p.m. UTC | #2 
On Thu,  7 Sep 2017 14:09:23 +0100
Bruce Richardson <bruce.richardson@intel.com> wrote:

> When DPDK is compiled on Ubuntu with extra warnings turned on, there is a
> warning about the return value from write() being unchecked. Rather than
> having builds disable the warning, which may mask other cases we do care
> about, we can add a dummy use of the return value in the code to silence it
> in this instance.
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>  lib/librte_cmdline/cmdline.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/librte_cmdline/cmdline.c b/lib/librte_cmdline/cmdline.c
> index a9c47be..d749165 100644
> --- a/lib/librte_cmdline/cmdline.c
> +++ b/lib/librte_cmdline/cmdline.c
> @@ -205,7 +205,8 @@ cmdline_printf(const struct cmdline *cl, const char *fmt, ...)
>  	}
>  	if (ret >= BUFSIZ)
>  		ret = BUFSIZ - 1;
> -	write(cl->s_out, buf, ret);
> +	ret = write(cl->s_out, buf, ret);
> +	(void)ret;

That is an ugly way to fix the warning.
If write fails, the user has probably logged out.
Olivier Matz Sept. 7, 2017, 7:46 p.m. UTC | #3 
On Thu, Sep 07, 2017 at 10:50:20AM -0700, Stephen Hemminger wrote:
> On Thu,  7 Sep 2017 14:09:23 +0100
> Bruce Richardson <bruce.richardson@intel.com> wrote:
> 
> > When DPDK is compiled on Ubuntu with extra warnings turned on, there is a
> > warning about the return value from write() being unchecked. Rather than
> > having builds disable the warning, which may mask other cases we do care
> > about, we can add a dummy use of the return value in the code to silence it
> > in this instance.
> > 
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > ---
> >  lib/librte_cmdline/cmdline.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/lib/librte_cmdline/cmdline.c b/lib/librte_cmdline/cmdline.c
> > index a9c47be..d749165 100644
> > --- a/lib/librte_cmdline/cmdline.c
> > +++ b/lib/librte_cmdline/cmdline.c
> > @@ -205,7 +205,8 @@ cmdline_printf(const struct cmdline *cl, const char *fmt, ...)
> >  	}
> >  	if (ret >= BUFSIZ)
> >  		ret = BUFSIZ - 1;
> > -	write(cl->s_out, buf, ret);
> > +	ret = write(cl->s_out, buf, ret);
> > +	(void)ret;
> 
> That is an ugly way to fix the warning.
> If write fails, the user has probably logged out.

Here, we are writing on stdout.
I don't think it's worst than calling printf() without checking the
return value.

By the way, it looks strange to this code is compiled, because
-D _GNU_SOURCE is passed to the compiler, and the code is:

	#ifdef _GNU_SOURCE
		...
	#else
		...
		write()
	#endif

It does not mean we shouldn't fix it, but maybe it hides something
else. Bruce, I bet you are testing your new build framework?

Olivier
Bruce Richardson Sept. 8, 2017, 9:01 a.m. UTC | #4 
On Thu, Sep 07, 2017 at 09:46:18PM +0200, Olivier MATZ wrote:
> On Thu, Sep 07, 2017 at 10:50:20AM -0700, Stephen Hemminger wrote:
> > On Thu,  7 Sep 2017 14:09:23 +0100
> > Bruce Richardson <bruce.richardson@intel.com> wrote:
> > 
> > > When DPDK is compiled on Ubuntu with extra warnings turned on, there is a
> > > warning about the return value from write() being unchecked. Rather than
> > > having builds disable the warning, which may mask other cases we do care
> > > about, we can add a dummy use of the return value in the code to silence it
> > > in this instance.
> > > 
> > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > > ---
> > >  lib/librte_cmdline/cmdline.c | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/lib/librte_cmdline/cmdline.c b/lib/librte_cmdline/cmdline.c
> > > index a9c47be..d749165 100644
> > > --- a/lib/librte_cmdline/cmdline.c
> > > +++ b/lib/librte_cmdline/cmdline.c
> > > @@ -205,7 +205,8 @@ cmdline_printf(const struct cmdline *cl, const char *fmt, ...)
> > >  	}
> > >  	if (ret >= BUFSIZ)
> > >  		ret = BUFSIZ - 1;
> > > -	write(cl->s_out, buf, ret);
> > > +	ret = write(cl->s_out, buf, ret);
> > > +	(void)ret;
> > 
> > That is an ugly way to fix the warning.
> > If write fails, the user has probably logged out.
> 
> Here, we are writing on stdout.
> I don't think it's worst than calling printf() without checking the
> return value.
> 
> By the way, it looks strange to this code is compiled, because
> -D _GNU_SOURCE is passed to the compiler, and the code is:
> 
> 	#ifdef _GNU_SOURCE
> 		...
> 	#else
> 		...
> 		write()
> 	#endif
> 
> It does not mean we shouldn't fix it, but maybe it hides something
> else. Bruce, I bet you are testing your new build framework?
> 
> Olivier

Yep, and meson has the -Wunused-result flag on by default, and on Ubuntu
write is marked as needing the result check. I don't like disabling
warnings, so I'd rather fix it this way, for the sake of on extra line
of ugliness. Open to better options though (other than not using Ubuntu,
which is fine with me, but some others seem to object to that :-) )
Van Haaren, Harry Sept. 15, 2017, 10:45 a.m. UTC | #5 
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Bruce Richardson
> Sent: Thursday, September 7, 2017 2:09 PM
> To: dev@dpdk.org
> Cc: Richardson, Bruce <bruce.richardson@intel.com>
> Subject: [dpdk-dev] [PATCH] cmdline: fix warning for unused return value
> 
> When DPDK is compiled on Ubuntu with extra warnings turned on, there is a
> warning about the return value from write() being unchecked. Rather than
> having builds disable the warning, which may mask other cases we do care
> about, we can add a dummy use of the return value in the code to silence it
> in this instance.
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---

A pragmatic fix for the compile warning at hand;

Acked-by: Harry van Haaren <harry.van.haaren@intel.com>


>  lib/librte_cmdline/cmdline.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/librte_cmdline/cmdline.c b/lib/librte_cmdline/cmdline.c
> index a9c47be..d749165 100644
> --- a/lib/librte_cmdline/cmdline.c
> +++ b/lib/librte_cmdline/cmdline.c
> @@ -205,7 +205,8 @@ cmdline_printf(const struct cmdline *cl, const char *fmt,
> ...)
>  	}
>  	if (ret >= BUFSIZ)
>  		ret = BUFSIZ - 1;
> -	write(cl->s_out, buf, ret);
> +	ret = write(cl->s_out, buf, ret);
> +	(void)ret;
>  	free(buf);
>  #endif
>  }
> --
> 2.7.4
Thomas Monjalon Oct. 5, 2017, 4:36 p.m. UTC | #6 
07/09/2017 15:49, Olivier MATZ:
> On Thu, Sep 07, 2017 at 02:09:23PM +0100, Bruce Richardson wrote:
> > When DPDK is compiled on Ubuntu with extra warnings turned on, there is a
> > warning about the return value from write() being unchecked. Rather than
> > having builds disable the warning, which may mask other cases we do care
> > about, we can add a dummy use of the return value in the code to silence it
> > in this instance.
> > 
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> 
> Acked-by: Olivier Matz <olivier.matz@6wind.com>

Applied, thanks
diff mbox

Patch

diff --git a/lib/librte_cmdline/cmdline.c b/lib/librte_cmdline/cmdline.c
index a9c47be..d749165 100644
--- a/lib/librte_cmdline/cmdline.c
+++ b/lib/librte_cmdline/cmdline.c
@@ -205,7 +205,8 @@  cmdline_printf(const struct cmdline *cl, const char *fmt, ...)
 	}
 	if (ret >= BUFSIZ)
 		ret = BUFSIZ - 1;
-	write(cl->s_out, buf, ret);
+	ret = write(cl->s_out, buf, ret);
+	(void)ret;
 	free(buf);
 #endif
 }