netdev - Re: [PATCH net-next 08/15] tools: ynl: wrap recv() + mnl_cb

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <e481d437-f55a-4895-ba1e-ccfac5dd0d9c@6wind.com>
Date: Fri, 23 Feb 2024 16:33:56 +0100
From: Nicolas Dichtel <nicolas.dichtel@...nd.com>
To: Jakub Kicinski <kuba@...nel.org>, davem@...emloft.net
Cc: netdev@...r.kernel.org, edumazet@...gle.com, pabeni@...hat.com,
 jiri@...nulli.us, sdf@...gle.com, donald.hunter@...il.com
Subject: Re: [PATCH net-next 08/15] tools: ynl: wrap recv() + mnl_cb_run2()
 into a single helper

Le 23/02/2024 à 00:56, Jakub Kicinski a écrit :
> All callers to mnl_cb_run2() call mnl_socket_recvfrom() right before.
> Wrap the two in a helper, take typed arguments (struct ynl_parse_arg),
> instead of hoping that all callers remember that parser error handling
> requires yarg.
> 
> Signed-off-by: Jakub Kicinski <kuba@...nel.org>
> ---
>  tools/net/ynl/lib/ynl.c | 56 +++++++++++++----------------------------
>  1 file changed, 18 insertions(+), 38 deletions(-)
> 
> diff --git a/tools/net/ynl/lib/ynl.c b/tools/net/ynl/lib/ynl.c
> index c9790257189c..96a209773ace 100644
> --- a/tools/net/ynl/lib/ynl.c
> +++ b/tools/net/ynl/lib/ynl.c
> @@ -491,6 +491,19 @@ int ynl_cb_null(const struct nlmsghdr *nlh, void *data)
>  	return MNL_CB_ERROR;
>  }
>  
> +static int ynl_sock_read_msgs(struct ynl_parse_arg *yarg, mnl_cb_t cb)
> +{
> +	struct ynl_sock *ys = yarg->ys;
> +	ssize_t len;
> +
> +	len = mnl_socket_recvfrom(ys->sock, ys->rx_buf, MNL_SOCKET_BUFFER_SIZE);
> +	if (len < 0)
It was '<=' before your patch. Not sure it's worth running mnl_cb_run2() with no
data, but this doesn't seem wrong.
Maybe a sentence in the commit description will be good to tell it's done on
purpose?

Acked-by: Nicolas Dichtel <nicolas.dichtel@...nd.com>