diff mbox

[dpdk-dev,v1] net/mlx4: fix missing initializers for old GCC

Message ID 1508848533-180885-1-git-send-email-motih@mellanox.com (mailing list archive)
State Accepted, archived
Delegated to: Ferruh Yigit
Headers

Checks

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

Commit Message

Moti Haimovsky Oct. 24, 2017, 12:35 p.m. UTC 
  This patch works around compilation issues seen on RHEL 7.2
using GCC 4.8.5:

   [...] In function 'mlx4_rss_init':
   [...]/mlx4_rxq.c:433:19: error: 'wq_num' may be used uninitialized
        in this function [-Werror=maybe-uninitialized]

Fixes: ff3397e90080 ("net/mlx4: relax Rx queue configuration order")

Signed-off-by: Moti Haimovsky <motih@mellanox.com>
---
 drivers/net/mlx4/mlx4_rxq.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Nélio Laranjeiro Oct. 24, 2017, 2:03 p.m. UTC | #1 
Hi Moti,

On Tue, Oct 24, 2017 at 03:35:33PM +0300, Moti Haimovsky wrote:
> This patch works around compilation issues seen on RHEL 7.2
> using GCC 4.8.5:
> 
>    [...] In function 'mlx4_rss_init':
>    [...]/mlx4_rxq.c:433:19: error: 'wq_num' may be used uninitialized
>         in this function [-Werror=maybe-uninitialized]
> 
> Fixes: ff3397e90080 ("net/mlx4: relax Rx queue configuration order")
> 
> Signed-off-by: Moti Haimovsky <motih@mellanox.com>
> ---
>  drivers/net/mlx4/mlx4_rxq.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/net/mlx4/mlx4_rxq.c b/drivers/net/mlx4/mlx4_rxq.c
> index fb28290..4c50077 100644
> --- a/drivers/net/mlx4/mlx4_rxq.c
> +++ b/drivers/net/mlx4/mlx4_rxq.c
> @@ -417,6 +417,8 @@ void mlx4_rss_detach(struct mlx4_rss *rss)
>  		if (wq) {
>  			wq_num = wq->wq_num;
>  			claim_zero(ibv_destroy_wq(wq));
> +		} else {
> +			 wq_num = 0; /* Shut up GCC 4.8 warnings. */
>  		}
>  		claim_zero(ibv_destroy_cq(cq));
>  		if (!wq) {

Why not initialising the wq_num at 0 directly instead of adding this
else branch?

Regards,
Adrien Mazarguil Oct. 24, 2017, 2:35 p.m. UTC | #2 
Hi Nelio,

On Tue, Oct 24, 2017 at 04:03:15PM +0200, Nélio Laranjeiro wrote:
> Hi Moti,
> 
> On Tue, Oct 24, 2017 at 03:35:33PM +0300, Moti Haimovsky wrote:
> > This patch works around compilation issues seen on RHEL 7.2
> > using GCC 4.8.5:
> > 
> >    [...] In function 'mlx4_rss_init':
> >    [...]/mlx4_rxq.c:433:19: error: 'wq_num' may be used uninitialized
> >         in this function [-Werror=maybe-uninitialized]
> > 
> > Fixes: ff3397e90080 ("net/mlx4: relax Rx queue configuration order")
> > 
> > Signed-off-by: Moti Haimovsky <motih@mellanox.com>
> > ---
> >  drivers/net/mlx4/mlx4_rxq.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/net/mlx4/mlx4_rxq.c b/drivers/net/mlx4/mlx4_rxq.c
> > index fb28290..4c50077 100644
> > --- a/drivers/net/mlx4/mlx4_rxq.c
> > +++ b/drivers/net/mlx4/mlx4_rxq.c
> > @@ -417,6 +417,8 @@ void mlx4_rss_detach(struct mlx4_rss *rss)
> >  		if (wq) {
> >  			wq_num = wq->wq_num;
> >  			claim_zero(ibv_destroy_wq(wq));
> > +		} else {
> > +			 wq_num = 0; /* Shut up GCC 4.8 warnings. */
> >  		}
> >  		claim_zero(ibv_destroy_cq(cq));
> >  		if (!wq) {
> 
> Why not initialising the wq_num at 0 directly instead of adding this
> else branch?

Actually that was my suggestion, it is done to highlight the code path
where buggy GCC versions choke on what they mistake for an uninitialized
variable. Initializing this variable earlier could possibly hide bugs
otherwise.

Patch is OK for me as is:

Acked-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
Adrien Mazarguil Oct. 24, 2017, 2:44 p.m. UTC | #3 
On Tue, Oct 24, 2017 at 03:35:33PM +0300, Moti Haimovsky wrote:
> This patch works around compilation issues seen on RHEL 7.2
> using GCC 4.8.5:
> 
>    [...] In function 'mlx4_rss_init':
>    [...]/mlx4_rxq.c:433:19: error: 'wq_num' may be used uninitialized
>         in this function [-Werror=maybe-uninitialized]
> 
> Fixes: ff3397e90080 ("net/mlx4: relax Rx queue configuration order")
> 
> Signed-off-by: Moti Haimovsky <motih@mellanox.com>
> ---
>  drivers/net/mlx4/mlx4_rxq.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/net/mlx4/mlx4_rxq.c b/drivers/net/mlx4/mlx4_rxq.c
> index fb28290..4c50077 100644
> --- a/drivers/net/mlx4/mlx4_rxq.c
> +++ b/drivers/net/mlx4/mlx4_rxq.c
> @@ -417,6 +417,8 @@ void mlx4_rss_detach(struct mlx4_rss *rss)
>  		if (wq) {
>  			wq_num = wq->wq_num;
>  			claim_zero(ibv_destroy_wq(wq));
> +		} else {
> +			 wq_num = 0; /* Shut up GCC 4.8 warnings. */

Wait, there's an indentation issue here, please remove unnecessary extra
space before "wq_num".

>  		}
>  		claim_zero(ibv_destroy_cq(cq));
>  		if (!wq) {
> -- 
> 1.8.3.1
>
Nélio Laranjeiro Oct. 24, 2017, 3:21 p.m. UTC | #4 
On Tue, Oct 24, 2017 at 04:35:05PM +0200, Adrien Mazarguil wrote:
> Hi Nelio,
> 
> On Tue, Oct 24, 2017 at 04:03:15PM +0200, Nélio Laranjeiro wrote:
> > Hi Moti,
> > 
> > On Tue, Oct 24, 2017 at 03:35:33PM +0300, Moti Haimovsky wrote:
> > > This patch works around compilation issues seen on RHEL 7.2
> > > using GCC 4.8.5:
> > > 
> > >    [...] In function 'mlx4_rss_init':
> > >    [...]/mlx4_rxq.c:433:19: error: 'wq_num' may be used uninitialized
> > >         in this function [-Werror=maybe-uninitialized]
> > > 
> > > Fixes: ff3397e90080 ("net/mlx4: relax Rx queue configuration order")
> > > 
> > > Signed-off-by: Moti Haimovsky <motih@mellanox.com>
> > > ---
> > >  drivers/net/mlx4/mlx4_rxq.c | 2 ++
> > >  1 file changed, 2 insertions(+)
> > > 
> > > diff --git a/drivers/net/mlx4/mlx4_rxq.c b/drivers/net/mlx4/mlx4_rxq.c
> > > index fb28290..4c50077 100644
> > > --- a/drivers/net/mlx4/mlx4_rxq.c
> > > +++ b/drivers/net/mlx4/mlx4_rxq.c
> > > @@ -417,6 +417,8 @@ void mlx4_rss_detach(struct mlx4_rss *rss)
> > >  		if (wq) {
> > >  			wq_num = wq->wq_num;
> > >  			claim_zero(ibv_destroy_wq(wq));
> > > +		} else {
> > > +			 wq_num = 0; /* Shut up GCC 4.8 warnings. */
> > >  		}
> > >  		claim_zero(ibv_destroy_cq(cq));
> > >  		if (!wq) {
> > 
> > Why not initialising the wq_num at 0 directly instead of adding this
> > else branch?
> 
> Actually that was my suggestion, it is done to highlight the code path
> where buggy GCC versions choke on what they mistake for an uninitialized
> variable. Initializing this variable earlier could possibly hide bugs
> otherwise.

I'll agree with this also.

> 
> Patch is OK for me as is:
> 
> Acked-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> 
> -- 
> Adrien Mazarguil
> 6WIND
Ferruh Yigit Oct. 24, 2017, 6:27 p.m. UTC | #5 
On 10/24/2017 7:35 AM, Adrien Mazarguil wrote:
> Hi Nelio,
> 
> On Tue, Oct 24, 2017 at 04:03:15PM +0200, Nélio Laranjeiro wrote:
>> Hi Moti,
>>
>> On Tue, Oct 24, 2017 at 03:35:33PM +0300, Moti Haimovsky wrote:
>>> This patch works around compilation issues seen on RHEL 7.2
>>> using GCC 4.8.5:
>>>
>>>    [...] In function 'mlx4_rss_init':
>>>    [...]/mlx4_rxq.c:433:19: error: 'wq_num' may be used uninitialized
>>>         in this function [-Werror=maybe-uninitialized]
>>>
>>> Fixes: ff3397e90080 ("net/mlx4: relax Rx queue configuration order")
>>>
>>> Signed-off-by: Moti Haimovsky <motih@mellanox.com>

> Acked-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>

Squashed into relevant commit in next-net, thanks.

(whitespace issue fixed while applying)
diff mbox

Patch

diff --git a/drivers/net/mlx4/mlx4_rxq.c b/drivers/net/mlx4/mlx4_rxq.c
index fb28290..4c50077 100644
--- a/drivers/net/mlx4/mlx4_rxq.c
+++ b/drivers/net/mlx4/mlx4_rxq.c
@@ -417,6 +417,8 @@  void mlx4_rss_detach(struct mlx4_rss *rss)
 		if (wq) {
 			wq_num = wq->wq_num;
 			claim_zero(ibv_destroy_wq(wq));
+		} else {
+			 wq_num = 0; /* Shut up GCC 4.8 warnings. */
 		}
 		claim_zero(ibv_destroy_cq(cq));
 		if (!wq) {