From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f53.google.com (mail-wm1-f53.google.com [209.85.128.53]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 8B3723B9D8A for ; Wed, 1 Apr 2026 08:59:57 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.53 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1775033999; cv=none; b=jEKWRN/RKi0Da8N90wqnEEjkmAdy70CEzf6NN6fGBXy3Wh+tCmj05QqHURZutgldiTYXvo8arNAH5ViMYaSP960gBcQmochgGMAyu9rehRyTc1GzRhMBQODUuMZaide5LABIXNVvRzmMiXZoaUeDTYZDqwrEWfc6XdGar5Bx+AQ= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1775033999; c=relaxed/simple; bh=y1bNRcq12Qxq27peoftGvFUoCas6fOGtr58zlt/Djlg=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=fpGNYp6z1vVo91cVzuQ8UV+XC7Yz/ATDqNksc2ySnTOHDwuHbfwwbN40JG06sL/tkD2nXqP9yPgdp2NzkN7gQTd8VcGFvDRiyMjsEYY0RrDB8LfU1Ubg4zRbjVlpkwsD5C9tiAyTcW0tgcsXdZ4KyZE5hkarXXzF6vlrYg8Qkw8= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=mZOGk5E7; arc=none smtp.client-ip=209.85.128.53 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="mZOGk5E7" Received: by mail-wm1-f53.google.com with SMTP id 5b1f17b1804b1-48374014a77so78882775e9.3 for ; Wed, 01 Apr 2026 01:59:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1775033996; x=1775638796; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=eWnWEKtH0ylGdVC9CRK6gwqNh+lWBWqZdfYCPLzOndM=; b=mZOGk5E7QLaJ0lsuF0OisKUDEDELHsp2P86BeA1GPPPi523r5Uo/cheXpSUUYeq3lA LbqdQbl1S+26SWSgjONM0vhVi+Vun+4Md86mAqR+HmQ2fqbXz674iko4Ll6dMcdhc1EV XeUbssfaAwe/CPi2ZrhxZpPTuJbtb97YmwTCI3xPbVt2UBU5c2aERh6UqqK+UB5XhGID LLQyDqYZ1ViKYmtd4ZKHGPMp9HtGTv0Js/sbGTb1S7gWXwlXDvzx56gmAWMuOSLmlEwX 3uFZ2VIQYT9QyUyAgmjfWVIFF6g5/j/ES7T3yobz4JTJRuZoX5WarjRt9C2a7L851PfU P+Tw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1775033996; x=1775638796; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-gg:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=eWnWEKtH0ylGdVC9CRK6gwqNh+lWBWqZdfYCPLzOndM=; b=dmvYl2tN8mL7q58Kli6nPLRm8JROWQsg/59SUnQ0R1qnMTUbOqAwJAxi28hT+Col+h QDyGB9zWpeU7cadhm04iEivstCJhi+eOgU/9U3R39pBmwPsSbLZsr9DGyeuPFoxpgHvB YNEui1Sfy/sPRzznHIUdls/LMUVguc0jAojOeOAxaWXh9VONVwX3TMvDJGbKGrNx1H7+ XgnNrvHK55Yp7VDcBrxY+9IUbjsdwXcdE8FNhTztOhn+G6W3cXzbtJR8LaGNZsntUnTe artYu63UAwepJeFJyssuPNWwLaOGVwP/UZSsirCbzB6ElhDflh8pHys2htEamK4Q6nOp bXOA== X-Forwarded-Encrypted: i=1; AJvYcCV692NVQqSXN89rhX32YlCBSAdsyLA9VQV2fyAZyA+W0eeOReihZl9TxIngxymBcIN1B8M1t2lKCU9VP1Q=@vger.kernel.org X-Gm-Message-State: AOJu0YyLOksS8j6ZJDJQhlWnjlLHaqvQlSQcD/I4H0Tm1pAIgrlAb5iB X64et+IcVrjGycZeMxsko9qKmcodoG/gA/01HzUs1RACW5CKSej6NQs3cgD4Ng== X-Gm-Gg: ATEYQzybyxLLAnIrBJPbvo1l+1fTvu43BQhX9hDS27IpD6EY3uuqObj3ttXnxiQrE6d 8VJ+uqWpGzg4ZxBT1wyeXUc4L6wTBeg4zfaYypyfhvddzJga8xgPihic2mK60RLifeg1ltQz7qL CkAdoyxmbZRyrRpAh/0SYH2OwhSO0t8QmG9M8tMGQGGjWgUz63kglICxOWVh4ojj+KymohCC5uJ ic7qgScaR69Fk3Tp5Cgd16Oydq9PMmA6SujpDu/EV8lcEBTPis4aLpzu2z4evfAh5W3aQaa/fkM x0v7e5kPO+ioFT3rzI2+PeAGaNM30CT7m3gz7wOGbOfGBDS8GnnyqeKgfrchKhrg9s59yMmzVQ+ 5NIA/0fL8e6k8JcqTAphr4AvjBCzeMdhsrAPSsycsALRB0Ut77nsZ6qQk1CUgZkYC8MsUG42qZv tFnnBlR8Q8nraYJng/6CI= X-Received: by 2002:a05:600c:828f:b0:485:3b50:fe54 with SMTP id 5b1f17b1804b1-4888356271fmr48534175e9.11.1775033995737; Wed, 01 Apr 2026 01:59:55 -0700 (PDT) Received: from localhost ([196.207.164.177]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-4887c561750sm125374045e9.2.2026.04.01.01.59.54 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 01 Apr 2026 01:59:55 -0700 (PDT) Date: Wed, 1 Apr 2026 11:59:52 +0300 From: Dan Carpenter To: Omer El Idrissi Cc: gregkh@linuxfoundation.org, linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org Subject: Re: [PATCH 5/5] staging: rtl8723bs: add separate label for freeing rtw_wdev Message-ID: References: <20260331153255.22764-1-omer.e.idrissi@gmail.com> <20260331153255.22764-6-omer.e.idrissi@gmail.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20260331153255.22764-6-omer.e.idrissi@gmail.com> On Tue, Mar 31, 2026 at 05:32:54PM +0200, Omer El Idrissi wrote: > The original function would "unregister" and "free" padapter->rtw_wdev > IF it ran into issues after "hardware operation functions" were set. > The problem is that rtw_wdev isn't even allocated at that point. > So separate the rtw_wdev_unregister and rtw_wdev_free code into it's own > label, add error check to rtw_wdev_alloc and move it up a bit. > When rtw_init_drv_sw fails goto free_wdev instead of free_hal_data. > This commit message makes it seem like this is a bugfix but really it's just a cleanup. > Signed-off-by: Omer El Idrissi > --- > drivers/staging/rtl8723bs/os_dep/sdio_intf.c | 14 ++++++++------ > 1 file changed, 8 insertions(+), 6 deletions(-) > > diff --git a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c > index aea9b4e19874..a088dae40a19 100644 > --- a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c > +++ b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c > @@ -268,12 +268,13 @@ static struct adapter *rtw_sdio_if1_init(struct dvobj_priv *dvobj, const struct > > /* 3 6. read efuse/eeprom data */ > rtw_hal_read_chip_info(padapter); > + > + if (rtw_wdev_alloc(padapter, dvobj_to_dev(dvobj))) > + goto free_hal_data; > > /* 3 7. init driver common data */ > if (rtw_init_drv_sw(padapter)) > - goto free_hal_data; > - > - rtw_wdev_alloc(padapter, dvobj_to_dev(dvobj)); > + goto free_wdev; > Then this part here re-orders things and adds new error checking which seems risky. Adding error checking for the rtw_wdev_alloc() function is probably the right thing, but it needs to be done separately and labeled with a Fixes tag. I don't know why you re-ordered things and it isn't explained in the commit message. > /* 3 8. get WLan MAC address */ > /* set mac addr */ > @@ -283,12 +284,13 @@ static struct adapter *rtw_sdio_if1_init(struct dvobj_priv *dvobj, const struct > > return padapter; > > -free_hal_data: > - kfree(padapter->HalData); > - > +free_wdev: > rtw_wdev_unregister(padapter->rtw_wdev); > rtw_wdev_free(padapter->rtw_wdev); > > +free_hal_data: > + kfree(padapter->HalData); > + I don't have a problem with re-ordering the cleanup. That stuff probably has never been tested and it's easy to review. https://staticthinking.wordpress.com/2022/04/28/free-the-last-thing-style/ So the commit message would say something like: "Generally, in a clean up ladder, we would free resources in the reverse order from how they are allocated. Here we potentially call rtw_wdev_unregister(padapter->rtw_wdev) and rtw_wdev_free(padapter->rtw_wdev) before the padapter->rtw_wdev has been allocated. This does not cause a problem because those functions check for NULL at the start and return directly, but it is a bit messy. Re-order the error handling in the cannonical way." regards, dan carpenter > free_adapter: > if (pnetdev) > rtw_free_netdev(pnetdev); > -- > 2.51.0 >