From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.2 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 317D7C433B4 for ; Wed, 28 Apr 2021 12:37:17 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id F2D31610FA for ; Wed, 28 Apr 2021 12:37:16 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237597AbhD1MiA (ORCPT ); Wed, 28 Apr 2021 08:38:00 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:36958 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237704AbhD1MiA (ORCPT ); Wed, 28 Apr 2021 08:38:00 -0400 Received: from mail-pj1-x1032.google.com (mail-pj1-x1032.google.com [IPv6:2607:f8b0:4864:20::1032]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id B3C83C061574 for ; Wed, 28 Apr 2021 05:37:15 -0700 (PDT) Received: by mail-pj1-x1032.google.com with SMTP id b14-20020a17090a6e0eb0290155c7f6a356so1981636pjk.0 for ; Wed, 28 Apr 2021 05:37:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pesu-pes-edu.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=6ARRtmav88LSZoCwmnUD6wYSwb7zXRvqm2Rfbvm63Co=; b=igvWqquW/p1mUnPsEEJHoKUHwvAA1Jr/Pu6wCtI6RwsxfJS/ZRbgv3wXQqJ7exU/1d Uklhp/aMi9h8BRiYA9WYceDLU5GPziuHoJRmMU6bkOjbrjoJi0d05nWXJeOdio0+5mrl soM4+tmlKnww6Oz1xfrJ7zfiiRnvRXhBuAbJqPSLusKOCnnR/WP4wUguy0YDvxPhJGmC 2C0+c3Tn3iDiEvhV4e8LzdVRc7N1AiR8IzpZouxu1hv3QL8K2pWHXZj0bGOJND625QYb i0IDwC6SEmbX/yQSAnNl3CEk1W+PeS9Ngxf6VuznrD92fotVx14rOB2l/F8AwyWNgObz HtOw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=6ARRtmav88LSZoCwmnUD6wYSwb7zXRvqm2Rfbvm63Co=; b=qwVMBvSOUjMKD5AMDnlzaixSO7/J7A34AWSNt9VgX9prhVNGetnVzlYYjGAFCtfdxd b1j70porOemU0kNCbZXCA9M9seUhp/ypmaPNdfUK0oOdY5VeofrM0SWToyNdFqPbaZ4b 6t5PZ45Dq8KJbsOa9G698xgJXWROtbmVJB30c/WKgQJoh2RSyrfz4nA6LEvVg9iEABYN gUELV5R4bf+l88VzPXl6AHDAoP+Rt95lXKwl4erhSrUAemSNgXKAy/wROd8O6M7cV/g7 NuZcXyJ2QL/IcXXgErNXyQRDRyxDHWH2QncrydHAUmtBtwBFOyrOdYiJVNrv2tj/urT6 FHPg== X-Gm-Message-State: AOAM531CYadPZAJ+v8eqbt6yWmAlAudaoNtwatnngKYSJWoFw5TKGhTv zA9hLwKQyTGEWrLlKDijr+OMTQ== X-Google-Smtp-Source: ABdhPJwxD0V11fzKlK9LEYltKzLMpxvMXlPXv3v5dK3ikG8RJ5EaVzb9JO+4R5M/bTtEUOL6+gQCOw== X-Received: by 2002:a17:90a:634a:: with SMTP id v10mr1739093pjs.33.1619613435199; Wed, 28 Apr 2021 05:37:15 -0700 (PDT) Received: from localhost ([2406:7400:73:8968:d957:fdf:9d7:1a08]) by smtp.gmail.com with ESMTPSA id l21sm5028377pfc.114.2021.04.28.05.37.14 (version=TLS1_2 cipher=ECDHE-ECDSA-CHACHA20-POLY1305 bits=256/256); Wed, 28 Apr 2021 05:37:14 -0700 (PDT) Date: Wed, 28 Apr 2021 18:07:09 +0530 From: bkkarthik To: Jaroslav Kysela Cc: Leon Romanovsky , Anupama K Patil , "Rafael J. Wysocki" , linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org, skhan@linuxfoundation.org, gregkh@linuxfoundation.org, kernelnewbies@kernelnewbies.org Subject: Re: [PATCH] drivers: pnp: proc.c: Handle errors while attaching devices Message-ID: <20210428123709.dbciscrm5qjr3bxa@burgerking> References: <20210424194301.jmsqpycvsm7izbk3@ubuntu> <20210426175031.w26ovnffjiow346h@burgerking> <59a5d631-6658-2034-06c4-467520b5b9f7@perex.cz> <19e8bd56-e24d-551e-9de2-57675541ee3f@perex.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <19e8bd56-e24d-551e-9de2-57675541ee3f@perex.cz> User-Agent: NeoMutt/20171215 Precedence: bulk List-ID: X-Mailing-List: linux-acpi@vger.kernel.org On 21/04/28 02:30PM, Jaroslav Kysela wrote: > Dne 28. 04. 21 v 14:21 Leon Romanovsky napsal(a): > > On Wed, Apr 28, 2021 at 02:04:49PM +0200, Jaroslav Kysela wrote: > >> Dne 26. 04. 21 v 19:50 bkkarthik napsal(a): > >>> On 21/04/26 08:04AM, Leon Romanovsky wrote: > >>>> On Sun, Apr 25, 2021 at 01:13:01AM +0530, Anupama K Patil wrote: > >>>>> isapnp_proc_init() does not look at the return value from > >>>>> isapnp_proc_attach_device(). Check for this return value in > >>>>> isapnp_proc_detach_device(). > >>>>> > >>>>> Cleanup in isapnp_proc_detach_device and > >>>>> isapnp_proc_detach_bus() for cleanup. > >>>>> > >>>>> Changed sprintf() to the kernel-space function scnprintf() as it returns > >>>>> the actual number of bytes written. > >>>>> > >>>>> Removed unnecessary variables de, e of type 'struct proc_dir_entry' to > >>>>> save memory. > >>>> > >>>> What exactly do you fix for such an old code? > >>> > >>> I was not aware that this code is so old. This fix was made after checkpatch reported assignment inside an if-statement. > >>> Please ignore this patch if th change is not necessary as the code is probably not being used anywhere :) > >>> > >>> Maybe the code has to be marked as obsolete in the MAINTAINERS file to prevent patches being sent? > >>> > >>>> > >>>>> > >>>>> Suggested-by: Shuah Khan > >>>>> Co-developed-by: B K Karthik > >>>>> Signed-off-by: B K Karthik > >>>>> Signed-off-by: Anupama K Patil > >>>>> --- > >>>>> drivers/pnp/isapnp/proc.c | 40 +++++++++++++++++++++++++++++---------- > >>>>> 1 file changed, 30 insertions(+), 10 deletions(-) > >>>>> > >>>>> diff --git a/drivers/pnp/isapnp/proc.c b/drivers/pnp/isapnp/proc.c > >>>>> index 785a796430fa..46ebc24175b7 100644 > >>>>> --- a/drivers/pnp/isapnp/proc.c > >>>>> +++ b/drivers/pnp/isapnp/proc.c > >>>>> @@ -54,34 +54,54 @@ static const struct proc_ops isapnp_proc_bus_proc_ops = { > >>>>> .proc_read = isapnp_proc_bus_read, > >>>>> }; > >>>>> > >>>>> +static int isapnp_proc_detach_device(struct pnp_dev *dev) > >>>>> +{ > >>>>> + proc_remove(dev->procent); > >>>>> + dev->procent = NULL; > >>>>> + return 0; > >>>>> +} > >>>>> + > >>>>> +static int isapnp_proc_detach_bus(struct pnp_card *bus) > >>>>> +{ > >>>>> + proc_remove(bus->procdir); > >>>>> + return 0; > >>>>> +} > >>>> > >>>> Please don't add one line functions that are called only once and have > >>>> return value that no one care about it. > >>> > >>> These were only intended for a clean-up job, the idea of this function came from how PCI handles procfs. > >>> Maybe those should be changed? > >> > >> Which code you refer? I see: > >> > >> for_each_pci_dev(dev) > >> pci_proc_attach_device(dev); > > > > He talks about isapnp_proc_detach_*() functions. > > But only this patch introduced those functions. The pci_proc_init() code does > not call pci_proc_detach_*() functions and ignores the allocation errors, too. The changes in this patch make isapnp_proc_init() look at the return value of isapnp_proc_attach_device() and call isapnp_proc_detach_device() if that returns an error code. > I don't think that this cleanup code is required. Oh okay! karthik > > Jaroslav > > -- > Jaroslav Kysela > Linux Sound Maintainer; ALSA Project; Red Hat, Inc. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.0 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 701ECC433B4 for ; Thu, 29 Apr 2021 18:20:56 +0000 (UTC) Received: from shelob.surriel.com (shelob.surriel.com [96.67.55.147]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 07CBB61450 for ; Thu, 29 Apr 2021 18:20:55 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 07CBB61450 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=pesu.pes.edu Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kernelnewbies-bounces@kernelnewbies.org Received: from localhost ([::1] helo=shelob.surriel.com) by shelob.surriel.com with esmtp (Exim 4.94) (envelope-from ) id 1lcBH2-0000G7-7F; Thu, 29 Apr 2021 14:20:28 -0400 Received: from mail-pj1-x102b.google.com ([2607:f8b0:4864:20::102b]) by shelob.surriel.com with esmtps (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.94) (envelope-from ) id 1lbjRM-0005uk-Ry for kernelnewbies@kernelnewbies.org; Wed, 28 Apr 2021 08:37:16 -0400 Received: by mail-pj1-x102b.google.com with SMTP id t2-20020a17090a0242b0290155433387beso3081910pje.1 for ; Wed, 28 Apr 2021 05:37:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pesu-pes-edu.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=6ARRtmav88LSZoCwmnUD6wYSwb7zXRvqm2Rfbvm63Co=; b=igvWqquW/p1mUnPsEEJHoKUHwvAA1Jr/Pu6wCtI6RwsxfJS/ZRbgv3wXQqJ7exU/1d Uklhp/aMi9h8BRiYA9WYceDLU5GPziuHoJRmMU6bkOjbrjoJi0d05nWXJeOdio0+5mrl soM4+tmlKnww6Oz1xfrJ7zfiiRnvRXhBuAbJqPSLusKOCnnR/WP4wUguy0YDvxPhJGmC 2C0+c3Tn3iDiEvhV4e8LzdVRc7N1AiR8IzpZouxu1hv3QL8K2pWHXZj0bGOJND625QYb i0IDwC6SEmbX/yQSAnNl3CEk1W+PeS9Ngxf6VuznrD92fotVx14rOB2l/F8AwyWNgObz HtOw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=6ARRtmav88LSZoCwmnUD6wYSwb7zXRvqm2Rfbvm63Co=; b=FW9gw8I0kzwFcpGFbbwARdhoh+XlY22VM5FGAF2Y/gi2bS71+hmrerek0VYidQC5Ag L81+XTUdOArmQI358j1Fc6BViLe4fD6iK2UhqYCjRFiEzZTvIAe0U9Hn4k1NyP1qD+kt OXhp0+zAwePaRls3KbiCMEYcYilSv2fOathybLlHMLd7mkXl6xXHg/GNmo4oK63CUYwK XNJrj3HQdXbXbxZkCe1p+6QUjZoewlY2Ato7lZobKn5wpi6oxLHlm9+lYraq+rVlbRSN ivB9peqyjetPBeyKI6I6CKziwt7ZpI+uMVvAxb20UuTzsM6za6jJOUaA8tcG2KlMSjwF tIJw== X-Gm-Message-State: AOAM533WJLKskSXIk/HIC7qp8frBG5jwCqp2Or3SkSVavvK9ZQc73Knb kPAV8bOmIh3+qdFhbejKlxIQXA== X-Google-Smtp-Source: ABdhPJwxD0V11fzKlK9LEYltKzLMpxvMXlPXv3v5dK3ikG8RJ5EaVzb9JO+4R5M/bTtEUOL6+gQCOw== X-Received: by 2002:a17:90a:634a:: with SMTP id v10mr1739093pjs.33.1619613435199; Wed, 28 Apr 2021 05:37:15 -0700 (PDT) Received: from localhost ([2406:7400:73:8968:d957:fdf:9d7:1a08]) by smtp.gmail.com with ESMTPSA id l21sm5028377pfc.114.2021.04.28.05.37.14 (version=TLS1_2 cipher=ECDHE-ECDSA-CHACHA20-POLY1305 bits=256/256); Wed, 28 Apr 2021 05:37:14 -0700 (PDT) Date: Wed, 28 Apr 2021 18:07:09 +0530 From: bkkarthik To: Jaroslav Kysela Subject: Re: [PATCH] drivers: pnp: proc.c: Handle errors while attaching devices Message-ID: <20210428123709.dbciscrm5qjr3bxa@burgerking> References: <20210424194301.jmsqpycvsm7izbk3@ubuntu> <20210426175031.w26ovnffjiow346h@burgerking> <59a5d631-6658-2034-06c4-467520b5b9f7@perex.cz> <19e8bd56-e24d-551e-9de2-57675541ee3f@perex.cz> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <19e8bd56-e24d-551e-9de2-57675541ee3f@perex.cz> User-Agent: NeoMutt/20171215 X-Mailman-Approved-At: Thu, 29 Apr 2021 14:16:15 -0400 Cc: Leon Romanovsky , gregkh@linuxfoundation.org, Anupama K Patil , "Rafael J. Wysocki" , linux-kernel@vger.kernel.org, linux-acpi@vger.kernel.org, kernelnewbies@kernelnewbies.org, skhan@linuxfoundation.org X-BeenThere: kernelnewbies@kernelnewbies.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: Learn about the Linux kernel List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: kernelnewbies-bounces@kernelnewbies.org On 21/04/28 02:30PM, Jaroslav Kysela wrote: > Dne 28. 04. 21 v 14:21 Leon Romanovsky napsal(a): > > On Wed, Apr 28, 2021 at 02:04:49PM +0200, Jaroslav Kysela wrote: > >> Dne 26. 04. 21 v 19:50 bkkarthik napsal(a): > >>> On 21/04/26 08:04AM, Leon Romanovsky wrote: > >>>> On Sun, Apr 25, 2021 at 01:13:01AM +0530, Anupama K Patil wrote: > >>>>> isapnp_proc_init() does not look at the return value from > >>>>> isapnp_proc_attach_device(). Check for this return value in > >>>>> isapnp_proc_detach_device(). > >>>>> > >>>>> Cleanup in isapnp_proc_detach_device and > >>>>> isapnp_proc_detach_bus() for cleanup. > >>>>> > >>>>> Changed sprintf() to the kernel-space function scnprintf() as it returns > >>>>> the actual number of bytes written. > >>>>> > >>>>> Removed unnecessary variables de, e of type 'struct proc_dir_entry' to > >>>>> save memory. > >>>> > >>>> What exactly do you fix for such an old code? > >>> > >>> I was not aware that this code is so old. This fix was made after checkpatch reported assignment inside an if-statement. > >>> Please ignore this patch if th change is not necessary as the code is probably not being used anywhere :) > >>> > >>> Maybe the code has to be marked as obsolete in the MAINTAINERS file to prevent patches being sent? > >>> > >>>> > >>>>> > >>>>> Suggested-by: Shuah Khan > >>>>> Co-developed-by: B K Karthik > >>>>> Signed-off-by: B K Karthik > >>>>> Signed-off-by: Anupama K Patil > >>>>> --- > >>>>> drivers/pnp/isapnp/proc.c | 40 +++++++++++++++++++++++++++++---------- > >>>>> 1 file changed, 30 insertions(+), 10 deletions(-) > >>>>> > >>>>> diff --git a/drivers/pnp/isapnp/proc.c b/drivers/pnp/isapnp/proc.c > >>>>> index 785a796430fa..46ebc24175b7 100644 > >>>>> --- a/drivers/pnp/isapnp/proc.c > >>>>> +++ b/drivers/pnp/isapnp/proc.c > >>>>> @@ -54,34 +54,54 @@ static const struct proc_ops isapnp_proc_bus_proc_ops = { > >>>>> .proc_read = isapnp_proc_bus_read, > >>>>> }; > >>>>> > >>>>> +static int isapnp_proc_detach_device(struct pnp_dev *dev) > >>>>> +{ > >>>>> + proc_remove(dev->procent); > >>>>> + dev->procent = NULL; > >>>>> + return 0; > >>>>> +} > >>>>> + > >>>>> +static int isapnp_proc_detach_bus(struct pnp_card *bus) > >>>>> +{ > >>>>> + proc_remove(bus->procdir); > >>>>> + return 0; > >>>>> +} > >>>> > >>>> Please don't add one line functions that are called only once and have > >>>> return value that no one care about it. > >>> > >>> These were only intended for a clean-up job, the idea of this function came from how PCI handles procfs. > >>> Maybe those should be changed? > >> > >> Which code you refer? I see: > >> > >> for_each_pci_dev(dev) > >> pci_proc_attach_device(dev); > > > > He talks about isapnp_proc_detach_*() functions. > > But only this patch introduced those functions. The pci_proc_init() code does > not call pci_proc_detach_*() functions and ignores the allocation errors, too. The changes in this patch make isapnp_proc_init() look at the return value of isapnp_proc_attach_device() and call isapnp_proc_detach_device() if that returns an error code. > I don't think that this cleanup code is required. Oh okay! karthik > > Jaroslav > > -- > Jaroslav Kysela > Linux Sound Maintainer; ALSA Project; Red Hat, Inc. _______________________________________________ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies