Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Message-ID: <58eb44800ec3aaafa0f3a88a6d046ac85aaa02fe.camel@perches.com>
Subject: Re: [PATCH] bonding/alb: return -ENOMEM when kmalloc failed
From:   Joe Perches <joe@perches.com>
To:     Yang Li <yang.lee@linux.alibaba.com>, j.vosburgh@gmail.com
Cc:     vfalico@gmail.com, andy@greyhouse.net, davem@davemloft.net,
        kuba@kernel.org, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org
Date:   Mon, 26 Apr 2021 06:53:12 -0700
In-Reply-To: <1619429620-52948-1-git-send-email-yang.lee@linux.alibaba.com>
References: <1619429620-52948-1-git-send-email-yang.lee@linux.alibaba.com>
Content-Type: text/plain; charset="ISO-8859-1"
User-Agent: Evolution 3.38.1-1 
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

On Mon, 2021-04-26 at 17:33 +0800, Yang Li wrote:
> The driver is using -1 instead of the -ENOMEM defined macro to
> specify that a buffer allocation failed. Using the correct error
> code is more intuitive.
> 
> Smatch tool warning:
> drivers/net/bonding/bond_alb.c:850 rlb_initialize() warn: returning -1
> instead of -ENOMEM is sloppy
> 
> No functional change, just more standardized.
[]
> diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
[]
> @@ -847,7 +847,7 @@ static int rlb_initialize(struct bonding *bond)
> ?
> 
> ?	new_hashtbl = kmalloc(size, GFP_KERNEL);
> ?	if (!new_hashtbl)
> -		return -1;
> +		return -ENOMEM;
> ?
> 
> ?	spin_lock_bh(&bond->mode_lock);
> ?
> 

Perhaps the bond_alb_initialize call here which uses the return
value from this function:

drivers/net/bonding/bond_main.c:                if (bond_alb_initialize(bond, (BOND_MODE(bond) == BOND_MODE_ALB)))
drivers/net/bonding/bond_main.c-                        return -ENOMEM;

should use a store/test/return style instead of a fixed value return.

						res = bond_alb_initialize(bond, BOND_MODE(bond) == BOND_MODE_ALB);
						if (res < 0)
							return res;