DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] net/enic: fix build with GCC 7.5
@ 2021-10-15 10:28 Ferruh Yigit
  2021-10-15 11:16 ` David Marchand
  0 siblings, 1 reply; 4+ messages in thread
From: Ferruh Yigit @ 2021-10-15 10:28 UTC (permalink / raw)
  To: John Daley, Hyong Youb Kim, Andrew Rybchenko
  Cc: Ferruh Yigit, dev, David Marchand

Build error:
../drivers/net/enic/enic_fm_flow.c: In function 'enic_fm_flow_parse':
../drivers/net/enic/enic_fm_flow.c:1467:24:
	error: 'dev' may be used uninitialized in this function
	[-Werror=maybe-uninitialized]
    struct rte_eth_dev *dev;
                        ^~~
../drivers/net/enic/enic_fm_flow.c:1580:24:
	error: 'dev' may be used uninitialized in this function
	[-Werror=maybe-uninitialized]
    struct rte_eth_dev *dev;
                        ^~~
../drivers/net/enic/enic_fm_flow.c:1599:24:
	error: 'dev' may be used uninitialized in this function
	[-Werror=maybe-uninitialized]
    struct rte_eth_dev *dev;
                        ^~~

Build error looks like false positive, but to silence the compiler
initializing the pointer with NULL.

Fixes: 7968917ccf64 ("net/enic: support meta flow actions to overrule destinations")

Reported-by: David Marchand <david.marchand@redhat.com>
Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
Cc: andrew.rybchenko@oktetlabs.ru

I am not sure about the solution and I don't have environment to verify,
sending this patch to verify the solution in CI and trigger discussion
for fix.
The patch is still in next-net, when a proper fix is found, it can be
squashed in next-net.
---
 drivers/net/enic/enic_fm_flow.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/enic/enic_fm_flow.c b/drivers/net/enic/enic_fm_flow.c
index 4092ff1f6154..2c60bb864e23 100644
--- a/drivers/net/enic/enic_fm_flow.c
+++ b/drivers/net/enic/enic_fm_flow.c
@@ -1464,7 +1464,7 @@ enic_fm_copy_action(struct enic_flowman *fm,
 		}
 		case RTE_FLOW_ACTION_TYPE_PORT_ID: {
 			const struct rte_flow_action_port_id *port;
-			struct rte_eth_dev *dev;
+			struct rte_eth_dev *dev = NULL;
 
 			if (!ingress && (overlap & PORT_ID)) {
 				ENICPMD_LOG(DEBUG, "cannot have multiple egress PORT_ID actions");
@@ -1577,7 +1577,7 @@ enic_fm_copy_action(struct enic_flowman *fm,
 		}
 		case RTE_FLOW_ACTION_TYPE_PORT_REPRESENTOR: {
 			const struct rte_flow_action_ethdev *ethdev;
-			struct rte_eth_dev *dev;
+			struct rte_eth_dev *dev = NULL;
 
 			ethdev = actions->conf;
 			ret = enic_fm_check_transfer_dst(enic, ethdev->port_id,
@@ -1596,7 +1596,7 @@ enic_fm_copy_action(struct enic_flowman *fm,
 		}
 		case RTE_FLOW_ACTION_TYPE_REPRESENTED_PORT: {
 			const struct rte_flow_action_ethdev *ethdev;
-			struct rte_eth_dev *dev;
+			struct rte_eth_dev *dev = NULL;
 
 			if (overlap & PORT_ID) {
 				ENICPMD_LOG(DEBUG, "cannot have multiple egress PORT_ID actions");
-- 
2.31.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [dpdk-dev] [PATCH] net/enic: fix build with GCC 7.5
  2021-10-15 10:28 [dpdk-dev] [PATCH] net/enic: fix build with GCC 7.5 Ferruh Yigit
@ 2021-10-15 11:16 ` David Marchand
  2021-10-18 11:16   ` Ferruh Yigit
  0 siblings, 1 reply; 4+ messages in thread
From: David Marchand @ 2021-10-15 11:16 UTC (permalink / raw)
  To: Ferruh Yigit; +Cc: John Daley, Hyong Youb Kim, Andrew Rybchenko, dev

On Fri, Oct 15, 2021 at 12:28 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>
> Build error:
> ../drivers/net/enic/enic_fm_flow.c: In function 'enic_fm_flow_parse':
> ../drivers/net/enic/enic_fm_flow.c:1467:24:
>         error: 'dev' may be used uninitialized in this function
>         [-Werror=maybe-uninitialized]
>     struct rte_eth_dev *dev;
>                         ^~~
> ../drivers/net/enic/enic_fm_flow.c:1580:24:
>         error: 'dev' may be used uninitialized in this function
>         [-Werror=maybe-uninitialized]
>     struct rte_eth_dev *dev;
>                         ^~~
> ../drivers/net/enic/enic_fm_flow.c:1599:24:
>         error: 'dev' may be used uninitialized in this function
>         [-Werror=maybe-uninitialized]
>     struct rte_eth_dev *dev;
>                         ^~~
>
> Build error looks like false positive, but to silence the compiler
> initializing the pointer with NULL.

enic_fm_check_transfer_dst() contains branches where dev is not set
and those branches return rte_flow_error_set return value.
dev is dereferenced later based on this return value == 0.

So the compiler probably thinks that rte_flow_error_set may return 0.
rte_flow_error_set is outside of compiler "view" at the moment it
compiles enic_fm_flow.c, so the compiler making the assumption this
function can return 0 is being prudent from my pov.


>
> Fixes: 7968917ccf64 ("net/enic: support meta flow actions to overrule destinations")
>
> Reported-by: David Marchand <david.marchand@redhat.com>
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>

Your fix looks good in any case.

Reviewed-by: David Marchand <david.marchand@redhat.com>


-- 
David Marchand


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [dpdk-dev] [PATCH] net/enic: fix build with GCC 7.5
  2021-10-15 11:16 ` David Marchand
@ 2021-10-18 11:16   ` Ferruh Yigit
  2021-10-18 15:55     ` Ferruh Yigit
  0 siblings, 1 reply; 4+ messages in thread
From: Ferruh Yigit @ 2021-10-18 11:16 UTC (permalink / raw)
  To: David Marchand; +Cc: John Daley, Hyong Youb Kim, Andrew Rybchenko, dev

On 10/15/2021 12:16 PM, David Marchand wrote:
> On Fri, Oct 15, 2021 at 12:28 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>>
>> Build error:
>> ../drivers/net/enic/enic_fm_flow.c: In function 'enic_fm_flow_parse':
>> ../drivers/net/enic/enic_fm_flow.c:1467:24:
>>          error: 'dev' may be used uninitialized in this function
>>          [-Werror=maybe-uninitialized]
>>      struct rte_eth_dev *dev;
>>                          ^~~
>> ../drivers/net/enic/enic_fm_flow.c:1580:24:
>>          error: 'dev' may be used uninitialized in this function
>>          [-Werror=maybe-uninitialized]
>>      struct rte_eth_dev *dev;
>>                          ^~~
>> ../drivers/net/enic/enic_fm_flow.c:1599:24:
>>          error: 'dev' may be used uninitialized in this function
>>          [-Werror=maybe-uninitialized]
>>      struct rte_eth_dev *dev;
>>                          ^~~
>>
>> Build error looks like false positive, but to silence the compiler
>> initializing the pointer with NULL.
> 
> enic_fm_check_transfer_dst() contains branches where dev is not set
> and those branches return rte_flow_error_set return value.
> dev is dereferenced later based on this return value == 0.
> 
> So the compiler probably thinks that rte_flow_error_set may return 0.
> rte_flow_error_set is outside of compiler "view" at the moment it
> compiles enic_fm_flow.c, so the compiler making the assumption this
> function can return 0 is being prudent from my pov.
> 
> 
>>
>> Fixes: 7968917ccf64 ("net/enic: support meta flow actions to overrule destinations")
>>
>> Reported-by: David Marchand <david.marchand@redhat.com>
>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> 
> Your fix looks good in any case.
> 
> Reviewed-by: David Marchand <david.marchand@redhat.com>
> 

Applied to dpdk-next-net/main, thanks.


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [dpdk-dev] [PATCH] net/enic: fix build with GCC 7.5
  2021-10-18 11:16   ` Ferruh Yigit
@ 2021-10-18 15:55     ` Ferruh Yigit
  0 siblings, 0 replies; 4+ messages in thread
From: Ferruh Yigit @ 2021-10-18 15:55 UTC (permalink / raw)
  To: David Marchand; +Cc: John Daley, Hyong Youb Kim, Andrew Rybchenko, dev

On 10/18/2021 12:16 PM, Ferruh Yigit wrote:
> On 10/15/2021 12:16 PM, David Marchand wrote:
>> On Fri, Oct 15, 2021 at 12:28 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>>>
>>> Build error:
>>> ../drivers/net/enic/enic_fm_flow.c: In function 'enic_fm_flow_parse':
>>> ../drivers/net/enic/enic_fm_flow.c:1467:24:
>>>          error: 'dev' may be used uninitialized in this function
>>>          [-Werror=maybe-uninitialized]
>>>      struct rte_eth_dev *dev;
>>>                          ^~~
>>> ../drivers/net/enic/enic_fm_flow.c:1580:24:
>>>          error: 'dev' may be used uninitialized in this function
>>>          [-Werror=maybe-uninitialized]
>>>      struct rte_eth_dev *dev;
>>>                          ^~~
>>> ../drivers/net/enic/enic_fm_flow.c:1599:24:
>>>          error: 'dev' may be used uninitialized in this function
>>>          [-Werror=maybe-uninitialized]
>>>      struct rte_eth_dev *dev;
>>>                          ^~~
>>>
>>> Build error looks like false positive, but to silence the compiler
>>> initializing the pointer with NULL.
>>
>> enic_fm_check_transfer_dst() contains branches where dev is not set
>> and those branches return rte_flow_error_set return value.
>> dev is dereferenced later based on this return value == 0.
>>
>> So the compiler probably thinks that rte_flow_error_set may return 0.
>> rte_flow_error_set is outside of compiler "view" at the moment it
>> compiles enic_fm_flow.c, so the compiler making the assumption this
>> function can return 0 is being prudent from my pov.
>>
>>
>>>
>>> Fixes: 7968917ccf64 ("net/enic: support meta flow actions to overrule destinations")
>>>
>>> Reported-by: David Marchand <david.marchand@redhat.com>
>>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>>
>> Your fix looks good in any case.
>>
>> Reviewed-by: David Marchand <david.marchand@redhat.com>
>>
> 
> Applied to dpdk-next-net/main, thanks.
> 

     Bugzilla ID: 812

Updated in next-net.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-10-18 15:55 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-15 10:28 [dpdk-dev] [PATCH] net/enic: fix build with GCC 7.5 Ferruh Yigit
2021-10-15 11:16 ` David Marchand
2021-10-18 11:16   ` Ferruh Yigit
2021-10-18 15:55     ` Ferruh Yigit

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).