From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751303AbdKYEkf (ORCPT ); Fri, 24 Nov 2017 23:40:35 -0500 Received: from aserp1040.oracle.com ([141.146.126.69]:16548 "EHLO aserp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750729AbdKYEke (ORCPT ); Fri, 24 Nov 2017 23:40:34 -0500 Date: Sat, 25 Nov 2017 07:40:14 +0300 From: Dan Carpenter To: ishraq.i.ashraf@gmail.com Cc: gregkh@linuxfoundation.org, himanshujha199640@gmail.com, goudapatilk@gmail.com, insafonov@gmail.com, devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org, Johannes Berg Subject: Re: [PATCH 2/2] staging: rtl8188eu: Fix private WEXT IOCTL calls Message-ID: <20171125044014.lfdwsrfgl4dz725d@mwanda> References: <1511400546-10875-1-git-send-email-ishraq.i.ashraf@gmail.com> <1511571155-12101-1-git-send-email-ishraq.i.ashraf@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1511571155-12101-1-git-send-email-ishraq.i.ashraf@gmail.com> User-Agent: NeoMutt/20170609 (1.8.3) X-Source-IP: userv0021.oracle.com [156.151.31.71] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Thanks for doing this. This needs to be folded in with the earlier patch you send and then resend it as a V2 patch. https://kernelnewbies.org/FirstKernelPatch#head-5c81b3c517a1d0bbc24f92594cb734e155fcbbcb I added Johannes to the CC list again because this is sort of his fault... To be honest, I'm a little bit puzzled. I would have thought Johannes's change would have made some code unused and that we could delete it now. I haven't looked at this. > > - if (ret == 0 && (copy_to_user(wrqu->data.pointer, param, wrqu->data.length))) > + if (pmlmepriv->htpriv.ht_option == false) > + psta->htpriv.ht_option = false; > + > + update_sta_info_apmode(padapter, psta); > + > + ret = 0; > + > + if (copy_to_user(wrqu->data.pointer, param, wrqu->data.length)) > ret = -EFAULT; > > - return ret; > + err_free_param: > + kfree(param); > + return ret; > } Please keep the success path and failure paths separate like I did in the example code that I wrote in my email. Don't indent the labels. It should look like this: update_sta_info_apmode(padapter, psta); if (copy_to_user(wrqu->data.pointer, param, wrqu->data.length)) { ret = -EFAULT; goto err_free_param; } kfree(param); return 0; err_free_param: kfree(param); return ret; regards, dan carpenter