diff mbox series

[ethtool] netlink: settings: Fix for wrong auto-negotiation state

Message ID 20241016035848.292603-1-mohan.prasad@microchip.com (mailing list archive)
State New, archived
Delegated to: Michal Kubecek
Headers show
Series [ethtool] netlink: settings: Fix for wrong auto-negotiation state | expand

Checks

Context Check Description
netdev/tree_selection success Not a local patch

Commit Message

Mohan Prasad J Oct. 16, 2024, 3:58 a.m. UTC
Auto-negotiation state in json format showed the
opposite state due to wrong comparison.
Fix for returning the correct auto-neg state implemented.

Signed-off-by: Mohan Prasad J <mohan.prasad@microchip.com>
---
 netlink/settings.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Michal Kubecek Nov. 4, 2024, 1:05 a.m. UTC | #1
On Wed, Oct 16, 2024 at 09:28:47AM +0530, Mohan Prasad J wrote:
> Auto-negotiation state in json format showed the
> opposite state due to wrong comparison.
> Fix for returning the correct auto-neg state implemented.
> 
> Signed-off-by: Mohan Prasad J <mohan.prasad@microchip.com>
> ---
>  netlink/settings.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/netlink/settings.c b/netlink/settings.c
> index dbfb520..a454bfb 100644
> --- a/netlink/settings.c
> +++ b/netlink/settings.c
> @@ -546,7 +546,7 @@ int linkmodes_reply_cb(const struct nlmsghdr *nlhdr, void *data)
>  						(autoneg == AUTONEG_DISABLE) ? "off" : "on");
>  		else
>  			print_bool(PRINT_JSON, "auto-negotiation", NULL,
> -				   autoneg == AUTONEG_DISABLE);
> +				   (autoneg == AUTONEG_DISABLE) ? false : true);
>  	}
>  	if (tb[ETHTOOL_A_LINKMODES_MASTER_SLAVE_CFG]) {
>  		uint8_t val;

The fix looks correct but isn't

	(autoneg == AUTONEG_DISABLE) ? false : true

just a more complicated way to say

	autoneg != AUTONEG_DISABLE

(and harder to read)?

Michal
Mohan Prasad J Nov. 4, 2024, 5 a.m. UTC | #2
Hello Michal,

Thank you for the comments.

> On Wed, Oct 16, 2024 at 09:28:47AM +0530, Mohan Prasad J wrote:
> > Auto-negotiation state in json format showed the opposite state due to
> > wrong comparison.
> > Fix for returning the correct auto-neg state implemented.
> >
> > Signed-off-by: Mohan Prasad J <mohan.prasad@microchip.com>
> > ---
> >  netlink/settings.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/netlink/settings.c b/netlink/settings.c index
> > dbfb520..a454bfb 100644
> > --- a/netlink/settings.c
> > +++ b/netlink/settings.c
> > @@ -546,7 +546,7 @@ int linkmodes_reply_cb(const struct nlmsghdr
> *nlhdr, void *data)
> >  						(autoneg ==
> AUTONEG_DISABLE) ? "off" : "on");
> >  		else
> >  			print_bool(PRINT_JSON, "auto-negotiation", NULL,
> > -				   autoneg == AUTONEG_DISABLE);
> > +				   (autoneg == AUTONEG_DISABLE) ? false :
> true);
> >  	}
> >  	if (tb[ETHTOOL_A_LINKMODES_MASTER_SLAVE_CFG]) {
> >  		uint8_t val;
> 
> The fix looks correct but isn't
> 
> 	(autoneg == AUTONEG_DISABLE) ? false : true
> 
> just a more complicated way to say
> 
> 	autoneg != AUTONEG_DISABLE
> 
> (and harder to read)?

You are right. (autoneg != AUTONEG_DISABLE) would be more simpler and easy to read. I will update it in the next version.

Thanks,
Mohan Prasad J
diff mbox series

Patch

diff --git a/netlink/settings.c b/netlink/settings.c
index dbfb520..a454bfb 100644
--- a/netlink/settings.c
+++ b/netlink/settings.c
@@ -546,7 +546,7 @@  int linkmodes_reply_cb(const struct nlmsghdr *nlhdr, void *data)
 						(autoneg == AUTONEG_DISABLE) ? "off" : "on");
 		else
 			print_bool(PRINT_JSON, "auto-negotiation", NULL,
-				   autoneg == AUTONEG_DISABLE);
+				   (autoneg == AUTONEG_DISABLE) ? false : true);
 	}
 	if (tb[ETHTOOL_A_LINKMODES_MASTER_SLAVE_CFG]) {
 		uint8_t val;