|
From: selvanair (C. Review) <ge...@op...> - 2025-10-27 20:38:28
|
Attention is currently required from: flichtenheld, plaisthos.
Hello plaisthos, flichtenheld,
I'd like you to do a code review.
Please visit
http://gerrit.openvpn.net/c/openvpn/+/1317?usp=email
to review the following change.
Change subject: pkcs11_management_id_get: Free certificate object after use
......................................................................
pkcs11_management_id_get: Free certificate object after use
Found by ZeroPath
Change-Id: I85320b8f1cfc02dfd561916e5637d9481edac59e
Signed-off-by: Selva Nair <sel...@gm...>
---
M src/openvpn/pkcs11.c
1 file changed, 3 insertions(+), 3 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/17/1317/1
diff --git a/src/openvpn/pkcs11.c b/src/openvpn/pkcs11.c
index ce64135..9afb181 100644
--- a/src/openvpn/pkcs11.c
+++ b/src/openvpn/pkcs11.c
@@ -436,9 +436,6 @@
{
pkcs11h_certificate_id_list_t id_list = NULL;
pkcs11h_certificate_id_list_t entry = NULL;
-#if 0 /* certificate_id seems to be unused -- JY */
- pkcs11h_certificate_id_t certificate_id = NULL;
-#endif
pkcs11h_certificate_t certificate = NULL;
CK_RV rv = CKR_OK;
unsigned char *certificate_blob = NULL;
@@ -548,6 +545,9 @@
pkcs11h_certificate_freeCertificateIdList(id_list);
id_list = NULL;
+ pkcs11h_certificate_freeCertificate(certificate);
+ certificate = NULL;
+
free(internal_id);
internal_id = NULL;
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1317?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I85320b8f1cfc02dfd561916e5637d9481edac59e
Gerrit-Change-Number: 1317
Gerrit-PatchSet: 1
Gerrit-Owner: selvanair <sel...@gm...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
|
|
From: plaisthos (C. Review) <ge...@op...> - 2025-10-27 21:19:39
|
Attention is currently required from: flichtenheld, selvanair. plaisthos has posted comments on this change by selvanair. ( http://gerrit.openvpn.net/c/openvpn/+/1317?usp=email ) Change subject: pkcs11_management_id_get: Free certificate object after use ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1317?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: I85320b8f1cfc02dfd561916e5637d9481edac59e Gerrit-Change-Number: 1317 Gerrit-PatchSet: 1 Gerrit-Owner: selvanair <sel...@gm...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: flichtenheld <fr...@li...> Gerrit-Attention: selvanair <sel...@gm...> Gerrit-Comment-Date: Mon, 27 Oct 2025 21:19:25 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes |
|
From: Gert D. <ge...@gr...> - 2025-10-27 21:28:02
|
From: Selva Nair <sel...@gm...> Found by ZeroPath Change-Id: I85320b8f1cfc02dfd561916e5637d9481edac59e Signed-off-by: Selva Nair <sel...@gm...> Acked-by: Arne Schwabe <arn...@rf...> Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1317 --- This change was reviewed on Gerrit and approved by at least one developer. I request to merge it to master. Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1317 This mail reflects revision 1 of this Change. Acked-by according to Gerrit (reflected above): Arne Schwabe <arn...@rf...> diff --git a/src/openvpn/pkcs11.c b/src/openvpn/pkcs11.c index ce64135..9afb181 100644 --- a/src/openvpn/pkcs11.c +++ b/src/openvpn/pkcs11.c @@ -436,9 +436,6 @@ { pkcs11h_certificate_id_list_t id_list = NULL; pkcs11h_certificate_id_list_t entry = NULL; -#if 0 /* certificate_id seems to be unused -- JY */ - pkcs11h_certificate_id_t certificate_id = NULL; -#endif pkcs11h_certificate_t certificate = NULL; CK_RV rv = CKR_OK; unsigned char *certificate_blob = NULL; @@ -548,6 +545,9 @@ pkcs11h_certificate_freeCertificateIdList(id_list); id_list = NULL; + pkcs11h_certificate_freeCertificate(certificate); + certificate = NULL; + free(internal_id); internal_id = NULL; |
|
From: Gert D. <ge...@gr...> - 2025-10-28 09:23:40
|
I have not reviewed/tested this beyond "BB compiles things, unit tests
(including pkcs11 on some of the instances) work". Stare-at-code looks
very trivially correct.
Your patch has been applied to the master branch.
commit 87f639c820b8365585fc825ee7ea9cdd86cf88c5
Author: Selva Nair
Date: Mon Oct 27 22:27:41 2025 +0100
pkcs11_management_id_get: Free certificate object after use
Signed-off-by: Selva Nair <sel...@gm...>
Acked-by: Arne Schwabe <arn...@rf...>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1317
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg33908.html
Signed-off-by: Gert Doering <ge...@gr...>
--
kind regards,
Gert Doering
|
|
From: cron2 (C. Review) <ge...@op...> - 2025-10-28 09:24:08
|
cron2 has uploaded a new patch set (#2) to the change originally created by selvanair. ( http://gerrit.openvpn.net/c/openvpn/+/1317?usp=email ) The following approvals got outdated and were removed: Code-Review+2 by plaisthos Change subject: pkcs11_management_id_get: Free certificate object after use ...................................................................... pkcs11_management_id_get: Free certificate object after use Found by ZeroPath Change-Id: I85320b8f1cfc02dfd561916e5637d9481edac59e Signed-off-by: Selva Nair <sel...@gm...> Acked-by: Arne Schwabe <arn...@rf...> Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1317 Message-Id: <202...@gr...> URL: https://www.mail-archive.com/ope...@li.../msg33908.html Signed-off-by: Gert Doering <ge...@gr...> --- M src/openvpn/pkcs11.c 1 file changed, 3 insertions(+), 3 deletions(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/17/1317/2 diff --git a/src/openvpn/pkcs11.c b/src/openvpn/pkcs11.c index ce64135..9afb181 100644 --- a/src/openvpn/pkcs11.c +++ b/src/openvpn/pkcs11.c @@ -436,9 +436,6 @@ { pkcs11h_certificate_id_list_t id_list = NULL; pkcs11h_certificate_id_list_t entry = NULL; -#if 0 /* certificate_id seems to be unused -- JY */ - pkcs11h_certificate_id_t certificate_id = NULL; -#endif pkcs11h_certificate_t certificate = NULL; CK_RV rv = CKR_OK; unsigned char *certificate_blob = NULL; @@ -548,6 +545,9 @@ pkcs11h_certificate_freeCertificateIdList(id_list); id_list = NULL; + pkcs11h_certificate_freeCertificate(certificate); + certificate = NULL; + free(internal_id); internal_id = NULL; -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1317?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: I85320b8f1cfc02dfd561916e5637d9481edac59e Gerrit-Change-Number: 1317 Gerrit-PatchSet: 2 Gerrit-Owner: selvanair <sel...@gm...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> |
|
From: cron2 (C. Review) <ge...@op...> - 2025-10-28 09:24:10
|
cron2 has submitted this change. ( http://gerrit.openvpn.net/c/openvpn/+/1317?usp=email ) Change subject: pkcs11_management_id_get: Free certificate object after use ...................................................................... pkcs11_management_id_get: Free certificate object after use Found by ZeroPath Change-Id: I85320b8f1cfc02dfd561916e5637d9481edac59e Signed-off-by: Selva Nair <sel...@gm...> Acked-by: Arne Schwabe <arn...@rf...> Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1317 Message-Id: <202...@gr...> URL: https://www.mail-archive.com/ope...@li.../msg33908.html Signed-off-by: Gert Doering <ge...@gr...> --- M src/openvpn/pkcs11.c 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/openvpn/pkcs11.c b/src/openvpn/pkcs11.c index ce64135..9afb181 100644 --- a/src/openvpn/pkcs11.c +++ b/src/openvpn/pkcs11.c @@ -436,9 +436,6 @@ { pkcs11h_certificate_id_list_t id_list = NULL; pkcs11h_certificate_id_list_t entry = NULL; -#if 0 /* certificate_id seems to be unused -- JY */ - pkcs11h_certificate_id_t certificate_id = NULL; -#endif pkcs11h_certificate_t certificate = NULL; CK_RV rv = CKR_OK; unsigned char *certificate_blob = NULL; @@ -548,6 +545,9 @@ pkcs11h_certificate_freeCertificateIdList(id_list); id_list = NULL; + pkcs11h_certificate_freeCertificate(certificate); + certificate = NULL; + free(internal_id); internal_id = NULL; -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1317?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: I85320b8f1cfc02dfd561916e5637d9481edac59e Gerrit-Change-Number: 1317 Gerrit-PatchSet: 2 Gerrit-Owner: selvanair <sel...@gm...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> |