[dpdk-dev,08/10] lpm6: fix compilation with -Og
Checks
Commit Message
The compilation with gcc-6.3.0 and EXTRA_CFLAGS=-Og gives the following
error:
CC rte_lpm6.o
rte_lpm6.c: In function ‘rte_lpm6_add_v1705’:
rte_lpm6.c:442:11: error: ‘tbl_next’ may be used uninitialized in
this function [-Werror=maybe-uninitialized]
if (!tbl[tbl_index].valid) {
^
rte_lpm6.c:521:29: note: ‘tbl_next’ was declared here
struct rte_lpm6_tbl_entry *tbl_next;
^~~~~~~~
This is a false positive from gcc. Fix it by initializing tbl_next
to NULL.
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
lib/librte_lpm/rte_lpm6.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Comments
On 9/11/2017 4:13 PM, Olivier Matz wrote:
> The compilation with gcc-6.3.0 and EXTRA_CFLAGS=-Og gives the following
> error:
>
> CC rte_lpm6.o
> rte_lpm6.c: In function ‘rte_lpm6_add_v1705’:
> rte_lpm6.c:442:11: error: ‘tbl_next’ may be used uninitialized in
> this function [-Werror=maybe-uninitialized]
> if (!tbl[tbl_index].valid) {
> ^
> rte_lpm6.c:521:29: note: ‘tbl_next’ was declared here
> struct rte_lpm6_tbl_entry *tbl_next;
> ^~~~~~~~
>
> This is a false positive from gcc. Fix it by initializing tbl_next
> to NULL.
This is hard to trace, and it seems there is a way to have it, not sure
practically possible:
rte_lpm6_add_v1705
add_step
if (depth <= bits_covered) {
...
return 0 <--- so tbl_next stays untouched.
tbl = tbl_next
add_step
tbl[tbl_index]
So same comment as previous, if there is a practically available path,
this patch makes it harder to find by hiding compiler warning, so adding
maintainer for decision.
>
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> ---
> lib/librte_lpm/rte_lpm6.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/librte_lpm/rte_lpm6.c b/lib/librte_lpm/rte_lpm6.c
> index b4a7df348..f9496fe1b 100644
> --- a/lib/librte_lpm/rte_lpm6.c
> +++ b/lib/librte_lpm/rte_lpm6.c
> @@ -518,7 +518,7 @@ rte_lpm6_add_v1705(struct rte_lpm6 *lpm, uint8_t *ip, uint8_t depth,
> uint32_t next_hop)
> {
> struct rte_lpm6_tbl_entry *tbl;
> - struct rte_lpm6_tbl_entry *tbl_next;
> + struct rte_lpm6_tbl_entry *tbl_next = NULL;
> int32_t rule_index;
> int status;
> uint8_t masked_ip[RTE_LPM6_IPV6_ADDR_SIZE];
>
On Fri, Oct 06, 2017 at 01:18:00AM +0100, Ferruh Yigit wrote:
> On 9/11/2017 4:13 PM, Olivier Matz wrote:
> > The compilation with gcc-6.3.0 and EXTRA_CFLAGS=-Og gives the following
> > error:
> >
> > CC rte_lpm6.o
> > rte_lpm6.c: In function ‘rte_lpm6_add_v1705’:
> > rte_lpm6.c:442:11: error: ‘tbl_next’ may be used uninitialized in
> > this function [-Werror=maybe-uninitialized]
> > if (!tbl[tbl_index].valid) {
> > ^
> > rte_lpm6.c:521:29: note: ‘tbl_next’ was declared here
> > struct rte_lpm6_tbl_entry *tbl_next;
> > ^~~~~~~~
> >
> > This is a false positive from gcc. Fix it by initializing tbl_next
> > to NULL.
>
> This is hard to trace, and it seems there is a way to have it, not sure
> practically possible:
>
> rte_lpm6_add_v1705
> add_step
> if (depth <= bits_covered) {
> ...
> return 0 <--- so tbl_next stays untouched.
> tbl = tbl_next
> add_step
> tbl[tbl_index]
>
>
> So same comment as previous, if there is a practically available path,
> this patch makes it harder to find by hiding compiler warning, so adding
> maintainer for decision.
>
>
+Cristian, who has more knowledge of this library than I.
I don't think there is an issue here, since there is an additional check
before the second and subsequent calls to add_step(). The condition
check on the loop in add_v1705 is:
"i < RTE_LPM6_IPV6_ADDR_SIZE && status == 1;"
The "return 0" sets status to 0, causing the loop to break, preventing
any more calls to "tbl = tbl_next".
Therefore I don't think this masks an issue, and the fix is ok.
In the absense of any other objections or comments on this from
Cristian:
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
On 9/11/2017 8:13 AM, Olivier Matz wrote:
> The compilation with gcc-6.3.0 and EXTRA_CFLAGS=-Og gives the following
> error:
>
> CC rte_lpm6.o
> rte_lpm6.c: In function ‘rte_lpm6_add_v1705’:
> rte_lpm6.c:442:11: error: ‘tbl_next’ may be used uninitialized in
> this function [-Werror=maybe-uninitialized]
> if (!tbl[tbl_index].valid) {
> ^
> rte_lpm6.c:521:29: note: ‘tbl_next’ was declared here
> struct rte_lpm6_tbl_entry *tbl_next;
> ^~~~~~~~
>
> This is a false positive from gcc. Fix it by initializing tbl_next
> to NULL.
>
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
Applied to dpdk-next-net/master, thanks.
@@ -518,7 +518,7 @@ rte_lpm6_add_v1705(struct rte_lpm6 *lpm, uint8_t *ip, uint8_t depth,
uint32_t next_hop)
{
struct rte_lpm6_tbl_entry *tbl;
- struct rte_lpm6_tbl_entry *tbl_next;
+ struct rte_lpm6_tbl_entry *tbl_next = NULL;
int32_t rule_index;
int status;
uint8_t masked_ip[RTE_LPM6_IPV6_ADDR_SIZE];