Message ID | 20230731203141.30044-1-jorge.lopez2@hp.com |
---|---|
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:918b:0:b0:3e4:2afc:c1 with SMTP id s11csp2276922vqg; Mon, 31 Jul 2023 14:32:11 -0700 (PDT) X-Google-Smtp-Source: APBJJlEmtCr/gzdChSl8BKQ421o9DnCVPEd5PHD061m6MLZj9NI9AHQyNft3EARn90np4CtN6RoO X-Received: by 2002:a17:902:ea95:b0:1b0:6038:2982 with SMTP id x21-20020a170902ea9500b001b060382982mr11054525plb.41.1690839130905; Mon, 31 Jul 2023 14:32:10 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1690839130; cv=none; d=google.com; s=arc-20160816; b=ttES2kfpx6ig8yOm47wf09/VL/cps4h16g3wMfn2svWVLTHYr5XUxVXz2qdcf4zj+v x4rtedoIEZK0b3FCnSAu2tkToCPXRiZhu7qBGxWZa+cnvq6vDNFSIVgKSGt7q7ogvU5m hwYeyvlTUY5o7Cg+0PdlQi+Gb5q25HdnZ10ahYiLHOIlM4yd+MY43wlv8BYu1pxFDMCJ jZvzA3r9nQOuXIwUpdmr4Vlg28iukbEqAJYGc0bpO37x6DkkY0yt9vN+PvrhYxsKbcjG IMapJOjoLQomUT+ellIFz3uNrcRuUU1ijglxHetUb5pJSXTAzoW8EZ0snLuJZe1cZtgc b3Pw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:to:from:dkim-signature; bh=9qCimbpyIfGRqpmV/wxa7E/CJwE+sDC0dWUhviOuN9M=; fh=SFEyBxM5WJZP1+1LTwWXXaFsrJtJ4d+K809teYU5Sxk=; b=Hz3qq8Ze0RLhnwqF+B8c5cwx4qJaXfeg3vRMd0zr5Kdf4JFB144LfslSDOM0Spk18m JzWcQ4+e9eSQEYX5qkugU26985SaCdKHjvqtDwPvMwmZeGt8ps0bsMoaZrlsPp8pr701 oKX79lnPD9m35glu9Yt6Xo5KFSygzHM1qJLdK5fEaYmQCE7vv7mtkJ7aGuVu2hlAyCR2 XFSUNJ4nZzrNlkwp6VfCdllX9MD8Un/QpZgI/cZFgzyq6S6+UzuMydCDUdQsp/Y/tWkV NSCwy5ax/nBkg1wiV63M7BCIxgdUonX7jBG0wTbo7z8rOVxg52UKUWveRYh1JdBJ7wPt daLQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20221208 header.b=dFjoSJYX; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id h23-20020a17090a9c1700b00261266bf8b8si9370799pjp.179.2023.07.31.14.31.57; Mon, 31 Jul 2023 14:32:10 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20221208 header.b=dFjoSJYX; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230426AbjGaUcQ (ORCPT <rfc822;dengxinlin2429@gmail.com> + 99 others); Mon, 31 Jul 2023 16:32:16 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51696 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229491AbjGaUcN (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Mon, 31 Jul 2023 16:32:13 -0400 Received: from mail-ot1-x332.google.com (mail-ot1-x332.google.com [IPv6:2607:f8b0:4864:20::332]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D2FED1BC6; Mon, 31 Jul 2023 13:31:43 -0700 (PDT) Received: by mail-ot1-x332.google.com with SMTP id 46e09a7af769-6bb29b9044dso4491417a34.1; Mon, 31 Jul 2023 13:31:43 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1690835503; x=1691440303; h=content-transfer-encoding:mime-version:message-id:date:subject:to :from:from:to:cc:subject:date:message-id:reply-to; bh=9qCimbpyIfGRqpmV/wxa7E/CJwE+sDC0dWUhviOuN9M=; b=dFjoSJYX10+MwJ7p+0J6i8Fljr+zv6PUYJFxStQMrrxpwimDPx7DW4KGwZzMiWCh2j bGAwCcnaAx/++gyRc7NneN8wU+zVdcg3WieMXXbIX64R67QksII3rbR/9DS0hzEUsxf6 YLrHOBfF9Ed3gOOM8j5VQEYi5gbUYT7rQyIK126plBiDCDDQIdT08HoXk7Z96BvcNeed 91+qStpS54igOLQmwPs7lUnRSjBTaa7HSm+YzcUeSG9aW0d1tY/WvsWXrEPKxsZ3lL5s a/g38OuKmkwJYNd2AzLKNYJEhMMqK/gGmPVW3k+S60Bl9oDalqzHMLxVEJyEVld4D38p 15fg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1690835503; x=1691440303; h=content-transfer-encoding:mime-version:message-id:date:subject:to :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=9qCimbpyIfGRqpmV/wxa7E/CJwE+sDC0dWUhviOuN9M=; b=iYWOqXogYhnebxWD7i+udXcZ3C9r4J0FbD9hdOvLOl6U9a68auMKLwt4Dv4TUGXQD0 yKsOF5H0/h2XNYhNPnJQsbbbFVNLVsI1Zq7kzSprrUCA8BiBOSpzMEJvn3zZUjBhsJsO HK+JTskCFhojUXwRICjNg9jnq1/Wj9voQH6xfnjvGPTxZQX1yJwl18sUo+hwdjnNQgpQ qLZ9jD4v9w28xCsMvOZdaU6xj+velmLbORZX1nC+rQ+VASPfswk9HxmGkmSwjdgFfLKK avpAt3H7DXj2+2luop0VTdfc6MwgzZ0wP+2km0SQ3wWJBvMwtrg6dUFfI+D5QWojeuCz fXTQ== X-Gm-Message-State: ABy/qLbT/qvv6yvx8GeqitmSbekp6Jk9CTGKmYkF6r18nJukl8mJ2U8+ gAXHnOCPsQMIpRIV6VpJNd0= X-Received: by 2002:a05:6830:1e52:b0:6b9:3ecb:db8f with SMTP id e18-20020a0568301e5200b006b93ecbdb8fmr11573145otj.10.1690835503110; Mon, 31 Jul 2023 13:31:43 -0700 (PDT) Received: from grumpy-VECTOR.hsd1.tx.comcast.net ([2601:2c3:480:7390:b620:ce9:47eb:26ab]) by smtp.gmail.com with ESMTPSA id g8-20020a9d6208000000b006b9848f8aa7sm4498841otj.45.2023.07.31.13.31.42 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 31 Jul 2023 13:31:42 -0700 (PDT) From: Jorge Lopez <jorgealtxwork@gmail.com> X-Google-Original-From: Jorge Lopez <jorge.lopez2@hp.com> To: hdegoede@redhat.com, platform-driver-x86@vger.kernel.org, linux-kernel@vger.kernel.org, thomas@t-8ch.de, ilpo.jarvinen@linux.intel.com, dan.carpenter@linaro.org Subject: [PATCH 0/8] hp-bioscfg: Overall fixes and code cleanup Date: Mon, 31 Jul 2023 15:31:33 -0500 Message-Id: <20230731203141.30044-1-jorge.lopez2@hp.com> X-Mailer: git-send-email 2.34.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM, RCVD_IN_DNSWL_BLOCKED,SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1772973332412212229 X-GMAIL-MSGID: 1772973332412212229 |
Series |
hp-bioscfg: Overall fixes and code cleanup
|
|
Message
Jorge Lopez
July 31, 2023, 8:31 p.m. UTC
Submit individual patches to address memory leaks and uninitialized variable errors. Addressed several review comments making the source code more readable. Removed duplicate use of variable in inner loop. Changes were tested with a HP EliteBook x360 1030 G3 Jorge Lopez (8): hp-bioscfg: Fix memory leaks in attribute packages hp-bioscfg: Fix uninitialized variable errors hp-bioscfg: Replace the word HACK from source code hp-bioscfg: Change how prerequisites size is evaluated hp-bioscfg: Change how order list size is evaluated hp-bioscfg: Change how enum possible values size is evaluated hp-bioscfg: Change how password encoding size is evaluated hp-bioscfg: Remove duplicate use of variable in inner loop .../x86/hp/hp-bioscfg/enum-attributes.c | 24 ++++++++---- .../x86/hp/hp-bioscfg/int-attributes.c | 15 +++++-- .../x86/hp/hp-bioscfg/order-list-attributes.c | 39 ++++++++++++------- .../x86/hp/hp-bioscfg/passwdobj-attributes.c | 27 +++++++++---- .../x86/hp/hp-bioscfg/string-attributes.c | 13 +++++-- 5 files changed, 82 insertions(+), 36 deletions(-) -- 2.34.1
Comments
These are fine. We still need to do something like this. Also we could just get rid of value_len completely. Nothing uses it. diff --git a/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c index cffc1c9ba3e77..6ba0e49e787ec 100644 --- a/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c +++ b/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c @@ -264,7 +264,7 @@ static int hp_populate_ordered_list_elements_from_package(union acpi_object *ord * Ordered list data is stored in hex and comma separated format * Convert the data and split it to show each element */ - ret = hp_convert_hexstr_to_str(str_value, value_len, &tmpstr, &tmp_len); + ret = hp_convert_hexstr_to_str(str_value, size, &tmpstr, &tmp_len); if (ret) goto exit_list;
I will submit a new patch replacing 'value_len' for 'size' in line 267 as indicated. 'value_len' is utilized earlier in the code so we cannot remove it completely from the function. On Tue, Aug 1, 2023 at 8:35 AM Dan Carpenter <dan.carpenter@linaro.org> wrote: > > These are fine. We still need to do something like this. Also we could > just get rid of value_len completely. Nothing uses it. > > diff --git a/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c > index cffc1c9ba3e77..6ba0e49e787ec 100644 > --- a/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c > +++ b/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c > @@ -264,7 +264,7 @@ static int hp_populate_ordered_list_elements_from_package(union acpi_object *ord > * Ordered list data is stored in hex and comma separated format > * Convert the data and split it to show each element > */ > - ret = hp_convert_hexstr_to_str(str_value, value_len, &tmpstr, &tmp_len); > + ret = hp_convert_hexstr_to_str(str_value, size, &tmpstr, &tmp_len); > if (ret) > goto exit_list; >
On Tue, Aug 01, 2023 at 09:52:05AM -0500, Jorge Lopez wrote: > I will submit a new patch replacing 'value_len' for 'size' in line 267 > as indicated. > 'value_len' is utilized earlier in the code so we cannot remove it > completely from the function. > After replacing size then it looks like this. $ grep value_len drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c int value_len = 0; &str_value, &value_len); &str_value, &value_len); It's a write only variable. regards, dan carpenter
Ok. Thanks for the clarification. I will remove 'value_len' and replace all its references with 'size'. On Tue, Aug 1, 2023 at 10:04 AM Dan Carpenter <dan.carpenter@linaro.org> wrote: > > On Tue, Aug 01, 2023 at 09:52:05AM -0500, Jorge Lopez wrote: > > I will submit a new patch replacing 'value_len' for 'size' in line 267 > > as indicated. > > 'value_len' is utilized earlier in the code so we cannot remove it > > completely from the function. > > > > After replacing size then it looks like this. > > $ grep value_len drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c > int value_len = 0; > &str_value, &value_len); > &str_value, &value_len); > > It's a write only variable. > > regards, > dan carpenter >
On Tue, Aug 01, 2023 at 10:10:05AM -0500, Jorge Lopez wrote: > Ok. Thanks for the clarification. I will remove 'value_len' and > replace all its references with 'size'. Ugh... No, that's worse than the original. Just leave value_len as is in that case. :P regards, dan carpenter
Hi, On 7/31/23 22:31, Jorge Lopez wrote: > Submit individual patches to address memory leaks and uninitialized > variable errors. > Addressed several review comments making the source code more readable. > Removed duplicate use of variable in inner loop. > > Changes were tested with a HP EliteBook x360 1030 G3 > > Jorge Lopez (8): > hp-bioscfg: Fix memory leaks in attribute packages > hp-bioscfg: Fix uninitialized variable errors > hp-bioscfg: Replace the word HACK from source code > hp-bioscfg: Change how prerequisites size is evaluated > hp-bioscfg: Change how order list size is evaluated > hp-bioscfg: Change how enum possible values size is evaluated > hp-bioscfg: Change how password encoding size is evaluated > hp-bioscfg: Remove duplicate use of variable in inner loop > > .../x86/hp/hp-bioscfg/enum-attributes.c | 24 ++++++++---- > .../x86/hp/hp-bioscfg/int-attributes.c | 15 +++++-- > .../x86/hp/hp-bioscfg/order-list-attributes.c | 39 ++++++++++++------- > .../x86/hp/hp-bioscfg/passwdobj-attributes.c | 27 +++++++++---- > .../x86/hp/hp-bioscfg/string-attributes.c | 13 +++++-- > 5 files changed, 82 insertions(+), 36 deletions(-) Thank you for your patch-series, I've applied the series to my review-hans branch: https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans Note it will show up in my review-hans branch once I've pushed my local branch there, which might take a while. Once I've run some tests on this branch the patches there will be added to the platform-drivers-x86/for-next branch and eventually will be included in the pdx86 pull-request to Linus for the next merge-window. Regards, Hans