[edk2] [PATCH 0/2] Refine the function comments.

[Qiu Shumin]

Qiu Shumin (2):
SecurityPkg: Refine the function comments.
CryptoPkg: Refine the function comments.

CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7VerifyNull.c | 2 +-
.../BaseCryptLibRuntimeCryptProtocol/Pk/CryptPkcs7VerifyNull.c | 2 +-
SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c | 6 ------
3 files changed, 2 insertions(+), 8 deletions(-)

[Qiu Shumin]

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Qiu Shumin <***@intel.com>
CC: Long, Qin <***@intel.com>
---
SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c | 6 ------
1 file changed, 6 deletions(-)

diff --git a/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c b/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c
index 6319255..13c9138 100644
--- a/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c
+++ b/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c
@@ -619,12 +619,6 @@ _Exit:
@param[in] AllowedDb Pointer to a list of pointers to EFI_SIGNATURE_LIST
structures which contains lists of X.509 certificates
of approved signers.
- @param[out] Content An optional caller-allocated buffer into which the
- function will copy the content of PKCS7 signedData.
- @param[in,out] ContentSize On input, points of the size in bytes of the optional
- buffer Content previously allocated by caller. On output,
- the value will contain the actual size of the content
- extracted from the signedData.

@retval EFI_SUCCESS The PKCS7 signedData is trusted.
@retval EFI_SECURITY_VIOLATION Fail to verify the signature in PKCS7 signedData.

[Jordan Justen]

Regarding the commit message, how about:

===

SecurityPkg/Pkcs7VerifyDxe: Cleanup P7CheckTrust function comments

The comments for P7CheckTrust described Content and ContentSize
parameters, but they don't exist.

===

Post by Qiu Shumin
Contributed-under: TianoCore Contribution Agreement 1.0

Did this Cc work with git send-email? I always worry when the comma
(,) is used in the name. Usually it is written as:

Cc: Qin Long <***@intel.com>

But, it might also work if you add quotes around the name:

Cc: "Long, Qin" <***@intel.com>

-Jordan

Post by Qiu Shumin
---
SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c b/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c
index 6319255..13c9138 100644
--- a/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c
+++ b/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c
@param[in] AllowedDb Pointer to a list of pointers to EFI_SIGNATURE_LIST
structures which contains lists of X.509 certificates
of approved signers.
- function will copy the content of PKCS7 signedData.
- buffer Content previously allocated by caller. On output,
- the value will contain the actual size of the content
- extracted from the signedData.
@retval EFI_SUCCESS The PKCS7 signedData is trusted.
@retval EFI_SECURITY_VIOLATION Fail to verify the signature in PKCS7 signedData.
--
1.9.5.msysgit.1
------------------------------------------------------------------------------
Don't Limit Your Business. Reach for the Cloud.
GigeNET's Cloud Solutions provide you with the tools and support that
you need to offload your IT needs and focus on growing your business.
Configured For All Businesses. Start Your Cloud Today.
https://www.gigenetcloud.com/
_______________________________________________
edk2-devel mailing list
https://lists.sourceforge.net/lists/listinfo/edk2-devel

[Long, Qin]

Reviewed-by: Qin Long < ***@intel.com>


Best Regards & Thanks,
LONG, Qin

-----Original Message-----
From: Qiu, Shumin
Sent: Tuesday, July 7, 2015 4:42 PM
To: edk2-***@lists.sourceforge.net
Cc: Qiu, Shumin; Long, Qin
Subject: [edk2][PATCH 1/2] SecurityPkg: Refine the function comments.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Qiu Shumin <***@intel.com>
CC: Long, Qin <***@intel.com>
---
SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c | 6 ------
1 file changed, 6 deletions(-)

diff --git a/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c b/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c
index 6319255..13c9138 100644
--- a/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c
+++ b/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c
@@ -619,12 +619,6 @@ _Exit:
@param[in] AllowedDb Pointer to a list of pointers to EFI_SIGNATURE_LIST
structures which contains lists of X.509 certificates
of approved signers.
- @param[out] Content An optional caller-allocated buffer into which the
- function will copy the content of PKCS7 signedData.
- @param[in,out] ContentSize On input, points of the size in bytes of the optional
- buffer Content previously allocated by caller. On output,
- the value will contain the actual size of the content
- extracted from the signedData.

@retval EFI_SUCCESS The PKCS7 signedData is trusted.
@retval EFI_SECURITY_VIOLATION Fail to verify the signature in PKCS7 signedData.

[Qiu Shumin]

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Qiu Shumin <***@intel.com>
CC: Long, Qin <***@intel.com>
---
CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7VerifyNull.c | 2 +-
.../Library/BaseCryptLibRuntimeCryptProtocol/Pk/CryptPkcs7VerifyNull.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7VerifyNull.c b/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7VerifyNull.c
index 567965d..09b92c7 100644
--- a/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7VerifyNull.c
+++ b/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7VerifyNull.c
@@ -114,7 +114,7 @@ Pkcs7Verify (
@retval TRUE The P7Data was correctly formatted for processing.
@retval FALSE The P7Data was not correctly formatted for processing.

-*/
+**/
BOOLEAN
EFIAPI
Pkcs7GetAttachedContent (
diff --git a/CryptoPkg/Library/BaseCryptLibRuntimeCryptProtocol/Pk/CryptPkcs7VerifyNull.c b/CryptoPkg/Library/BaseCryptLibRuntimeCryptProtocol/Pk/CryptPkcs7VerifyNull.c
index 567965d..09b92c7 100644
--- a/CryptoPkg/Library/BaseCryptLibRuntimeCryptProtocol/Pk/CryptPkcs7VerifyNull.c
+++ b/CryptoPkg/Library/BaseCryptLibRuntimeCryptProtocol/Pk/CryptPkcs7VerifyNull.c
@@ -114,7 +114,7 @@ Pkcs7Verify (
@retval TRUE The P7Data was correctly formatted for processing.
@retval FALSE The P7Data was not correctly formatted for processing.

-*/
+**/
BOOLEAN
EFIAPI
Pkcs7GetAttachedContent (

[Long, Qin]

Reviewed-by: Qin Long < ***@intel.com>


Best Regards & Thanks,
LONG, Qin

-----Original Message-----
From: Qiu, Shumin
Sent: Tuesday, July 7, 2015 4:42 PM
To: edk2-***@lists.sourceforge.net
Cc: Qiu, Shumin; Long, Qin
Subject: [edk2][PATCH 2/2] CryptoPkg: Refine the function comments.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Qiu Shumin <***@intel.com>
CC: Long, Qin <***@intel.com>
---
CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7VerifyNull.c | 2 +-
.../Library/BaseCryptLibRuntimeCryptProtocol/Pk/CryptPkcs7VerifyNull.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7VerifyNull.c b/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7VerifyNull.c
index 567965d..09b92c7 100644
--- a/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7VerifyNull.c
+++ b/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7VerifyNull.c
@@ -114,7 +114,7 @@ Pkcs7Verify (
@retval TRUE The P7Data was correctly formatted for processing.
@retval FALSE The P7Data was not correctly formatted for processing.

-*/
+**/
BOOLEAN
EFIAPI
Pkcs7GetAttachedContent (
diff --git a/CryptoPkg/Library/BaseCryptLibRuntimeCryptProtocol/Pk/CryptPkcs7VerifyNull.c b/CryptoPkg/Library/BaseCryptLibRuntimeCryptProtocol/Pk/CryptPkcs7VerifyNull.c
index 567965d..09b92c7 100644
--- a/CryptoPkg/Library/BaseCryptLibRuntimeCryptProtocol/Pk/CryptPkcs7VerifyNull.c
+++ b/CryptoPkg/Library/BaseCryptLibRuntimeCryptProtocol/Pk/CryptPkcs7VerifyNull.c
@@ -114,7 +114,7 @@ Pkcs7Verify (
@retval TRUE The P7Data was correctly formatted for processing.
@retval FALSE The P7Data was not correctly formatted for processing.

-*/
+**/
BOOLEAN
EFIAPI
Pkcs7GetAttachedContent (

[Long, Qin]

Shumin,

And please update the commit log for better explanation per Leif's suggestion.


Best Regards & Thanks,
LONG, Qin

-----Original Message-----
From: Long, Qin [mailto:***@intel.com]
Sent: Monday, July 13, 2015 4:49 PM
To: Qiu, Shumin; edk2-***@lists.sourceforge.net
Subject: Re: [edk2] [PATCH 2/2] CryptoPkg: Refine the function comments.

Reviewed-by: Qin Long < ***@intel.com>


Best Regards & Thanks,
LONG, Qin

-----Original Message-----
From: Qiu, Shumin
Sent: Tuesday, July 7, 2015 4:42 PM
To: edk2-***@lists.sourceforge.net
Cc: Qiu, Shumin; Long, Qin
Subject: [edk2][PATCH 2/2] CryptoPkg: Refine the function comments.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Qiu Shumin <***@intel.com>
CC: Long, Qin <***@intel.com>
---
CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7VerifyNull.c | 2 +-
.../Library/BaseCryptLibRuntimeCryptProtocol/Pk/CryptPkcs7VerifyNull.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7VerifyNull.c b/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7VerifyNull.c
index 567965d..09b92c7 100644
--- a/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7VerifyNull.c
+++ b/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7VerifyNull.c
@@ -114,7 +114,7 @@ Pkcs7Verify (
@retval TRUE The P7Data was correctly formatted for processing.
@retval FALSE The P7Data was not correctly formatted for processing.

-*/
+**/
BOOLEAN
EFIAPI
Pkcs7GetAttachedContent (
diff --git a/CryptoPkg/Library/BaseCryptLibRuntimeCryptProtocol/Pk/CryptPkcs7VerifyNull.c b/CryptoPkg/Library/BaseCryptLibRuntimeCryptProtocol/Pk/CryptPkcs7VerifyNull.c
index 567965d..09b92c7 100644
--- a/CryptoPkg/Library/BaseCryptLibRuntimeCryptProtocol/Pk/CryptPkcs7VerifyNull.c
+++ b/CryptoPkg/Library/BaseCryptLibRuntimeCryptProtocol/Pk/CryptPkcs7VerifyNull.c
@@ -114,7 +114,7 @@ Pkcs7Verify (
@retval TRUE The P7Data was correctly formatted for processing.
@retval FALSE The P7Data was not correctly formatted for processing.

-*/
+**/
BOOLEAN
EFIAPI
Pkcs7GetAttachedContent (
--
1.9.5.msysgit.1


------------------------------------------------------------------------------
Don't Limit Your Business. Reach for the Cloud.
GigeNET's Cloud Solutions provide you with the tools and support that
you need to offload your IT needs and focus on growing your business.
Configured For All Businesses. Start Your Cloud Today.
https://www.gigenetcloud.com/
_______________________________________________
edk2-devel mailing list
edk2-***@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel

[Leif Lindholm]

Hi Qui,

Post by Qiu Shumin
SecurityPkg: Refine the function comments.
CryptoPkg: Refine the function comments.
CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7VerifyNull.c | 2 +-
.../BaseCryptLibRuntimeCryptProtocol/Pk/CryptPkcs7VerifyNull.c | 2 +-
SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c | 6 ------
3 files changed, 2 insertions(+), 8 deletions(-)
--
1.9.5.msysgit.1

Neither this cover letter or the patches themselves explain what these
changes do.

For 1/2, documentation of some parameters (that are not present in the
function prototype) is removed.
This does not require a huge lot of documentation, but something like
"Delete description of non-existent parameters Content and ContentSize
from P7CheckTrust() description." would be useful to have.

For 2/2, /* */ is changed to /** **/, presumably as a command to
doxygen? If so, this would be useful to state in the commit message
for that patch.

Best Regards,

Leif