netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next] tools: ynl: avoid "use of uninitialized variable" false positive in generated code
@ 2025-09-15 14:44 Vladimir Oltean
  2025-09-15 21:40 ` Jakub Kicinski
  2025-09-16 15:38 ` patchwork-bot+netdevbpf
  0 siblings, 2 replies; 3+ messages in thread
From: Vladimir Oltean @ 2025-09-15 14:44 UTC (permalink / raw)
  To: netdev
  Cc: Donald Hunter, Jakub Kicinski, David S. Miller, Eric Dumazet,
	Paolo Abeni, Simon Horman

With indexed-array types such as "ops" from
Documentation/netlink/specs/nlctrl.yaml, the generator creates code
such as:

int nlctrl_getfamily_rsp_parse(const struct nlmsghdr *nlh,
			       struct ynl_parse_arg *yarg)
{
	struct nlctrl_getfamily_rsp *dst;
	const struct nlattr *attr_ops;
	const struct nlattr *attr;
	struct ynl_parse_arg parg;
	unsigned int n_ops = 0;
	int i;

	...

	ynl_attr_for_each(attr, nlh, yarg->ys->family->hdr_len) {
		unsigned int type = ynl_attr_type(attr);

		if (type == CTRL_ATTR_FAMILY_ID) {
			...
		} else if (type == CTRL_ATTR_OPS) {
			const struct nlattr *attr2;

			attr_ops = attr;
			ynl_attr_for_each_nested(attr2, attr) {
				if (ynl_attr_validate(yarg, attr2))
					return YNL_PARSE_CB_ERROR;
				n_ops++;
			}
		} else {
			...
		}
	}
	if (n_ops) {
		dst->ops = calloc(n_ops, sizeof(*dst->ops));
		dst->_count.ops = n_ops;
		i = 0;
		parg.rsp_policy = &nlctrl_op_attrs_nest;
		ynl_attr_for_each_nested(attr, attr_ops) {
			...
		}
	}

	return YNL_PARSE_CB_OK;
}

It is clear that due to the sequential nature of code execution, when
n_ops (initially zero) is incremented, attr_ops is also assigned from
the value of "attr" (the current iterator).

But some compilers, like gcc version 12.2.0 (Debian 12.2.0-14+deb12u1)
as distributed by Debian Bookworm, seem to be not sophisticated enough
to see this, and fail to compile (warnings treated as errors):

In file included from ../lib/ynl.h:10,
                 from nlctrl-user.c:9:
In function ‘ynl_attr_data_end’,
    inlined from ‘nlctrl_getfamily_rsp_parse’ at nlctrl-user.c:427:3:
../lib/ynl-priv.h:209:44: warning: ‘attr_ops’ may be used uninitialized [-Wmaybe-uninitialized]
  209 |         return (char *)ynl_attr_data(attr) + ynl_attr_data_len(attr);
      |                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~
nlctrl-user.c: In function ‘nlctrl_getfamily_rsp_parse’:
nlctrl-user.c:341:30: note: ‘attr_ops’ was declared here
  341 |         const struct nlattr *attr_ops;
      |                              ^~~~~~~~

It is a pity that we have to do this, but I see no other way than to
suppress the false positive by appeasing the compiler and initializing
the "*attr_{aspec.c_name}" variable with a bogus value (NULL). This will
never be used - at runtime it will always be overwritten when
"n_{struct[anest].c_name}" is non-zero.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 tools/net/ynl/pyynl/ynl_gen_c.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/net/ynl/pyynl/ynl_gen_c.py b/tools/net/ynl/pyynl/ynl_gen_c.py
index fb7e03805a11..0155695d1842 100755
--- a/tools/net/ynl/pyynl/ynl_gen_c.py
+++ b/tools/net/ynl/pyynl/ynl_gen_c.py
@@ -2107,10 +2107,10 @@ def _multi_parse(ri, struct, init_lines, local_vars):
     for arg, aspec in struct.member_list():
         if aspec['type'] == 'indexed-array' and 'sub-type' in aspec:
             if aspec["sub-type"] in {'binary', 'nest'}:
-                local_vars.append(f'const struct nlattr *attr_{aspec.c_name};')
+                local_vars.append(f'const struct nlattr *attr_{aspec.c_name} = NULL;')
                 array_nests.add(arg)
             elif aspec['sub-type'] in scalars:
-                local_vars.append(f'const struct nlattr *attr_{aspec.c_name};')
+                local_vars.append(f'const struct nlattr *attr_{aspec.c_name} = NULL;')
                 array_nests.add(arg)
             else:
                 raise Exception(f'Not supported sub-type {aspec["sub-type"]}')
-- 
2.34.1


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

* Re: [PATCH net-next] tools: ynl: avoid "use of uninitialized variable" false positive in generated code
  2025-09-15 14:44 [PATCH net-next] tools: ynl: avoid "use of uninitialized variable" false positive in generated code Vladimir Oltean
@ 2025-09-15 21:40 ` Jakub Kicinski
  2025-09-16 15:38 ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 3+ messages in thread
From: Jakub Kicinski @ 2025-09-15 21:40 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: netdev, Donald Hunter, David S. Miller, Eric Dumazet, Paolo Abeni,
	Simon Horman

On Mon, 15 Sep 2025 17:44:14 +0300 Vladimir Oltean wrote:
> It is a pity that we have to do this, but I see no other way than to
> suppress the false positive by appeasing the compiler and initializing
> the "*attr_{aspec.c_name}" variable with a bogus value (NULL). This will
> never be used - at runtime it will always be overwritten when
> "n_{struct[anest].c_name}" is non-zero.

Yeah, I'm hitting this on CentOS, too :(

Reviewed-by: Jakub Kicinski <kuba@kernel.org>

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

* Re: [PATCH net-next] tools: ynl: avoid "use of uninitialized variable" false positive in generated code
  2025-09-15 14:44 [PATCH net-next] tools: ynl: avoid "use of uninitialized variable" false positive in generated code Vladimir Oltean
  2025-09-15 21:40 ` Jakub Kicinski
@ 2025-09-16 15:38 ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 3+ messages in thread
From: patchwork-bot+netdevbpf @ 2025-09-16 15:38 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: netdev, donald.hunter, kuba, davem, edumazet, pabeni, horms

Hello:

This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Mon, 15 Sep 2025 17:44:14 +0300 you wrote:
> With indexed-array types such as "ops" from
> Documentation/netlink/specs/nlctrl.yaml, the generator creates code
> such as:
> 
> int nlctrl_getfamily_rsp_parse(const struct nlmsghdr *nlh,
> 			       struct ynl_parse_arg *yarg)
> {
> 	struct nlctrl_getfamily_rsp *dst;
> 	const struct nlattr *attr_ops;
> 	const struct nlattr *attr;
> 	struct ynl_parse_arg parg;
> 	unsigned int n_ops = 0;
> 	int i;
> 
> [...]

Here is the summary with links:
  - [net-next] tools: ynl: avoid "use of uninitialized variable" false positive in generated code
    https://git.kernel.org/netdev/net-next/c/a6824f65c996

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



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

end of thread, other threads:[~2025-09-16 15:38 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-15 14:44 [PATCH net-next] tools: ynl: avoid "use of uninitialized variable" false positive in generated code Vladimir Oltean
2025-09-15 21:40 ` Jakub Kicinski
2025-09-16 15:38 ` patchwork-bot+netdevbpf

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).