Re: [PATCH] sign-file: correct error handling

Re: [PATCH] sign-file: correct error handling

[Salvatore Bonaccorso]

Hi Ansgar,

On Sun, Jul 03, 2022 at 11:17:50AM +0200, Ansgar Burchardt wrote:
> The functions CMS_final, i2d_CMS_bio_stream, i2d_PKCS7_bio and
> BIO_free all return 1 for success or 0 for failure.
> 
> Signed-off-by: Ansgar Burchardt <ansgar@43-1.org>
> ---
>  scripts/sign-file.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/scripts/sign-file.c b/scripts/sign-file.c
> index 7434e9ea926e..4889f919ff8a 100644
> --- a/scripts/sign-file.c
> +++ b/scripts/sign-file.c
> @@ -322,7 +322,7 @@ int main(int argc, char **argv)
>  				     CMS_NOSMIMECAP | use_keyid |
>  				     use_signed_attrs),
>  		    "CMS_add1_signer");
> -		ERR(CMS_final(cms, bm, NULL, CMS_NOCERTS | CMS_BINARY) < 0,
> +		ERR(!CMS_final(cms, bm, NULL, CMS_NOCERTS | CMS_BINARY),
>  		    "CMS_final");
>  
>  #else
> @@ -341,10 +341,10 @@ int main(int argc, char **argv)
>  			b = BIO_new_file(sig_file_name, "wb");
>  			ERR(!b, "%s", sig_file_name);
>  #ifndef USE_PKCS7
> -			ERR(i2d_CMS_bio_stream(b, cms, NULL, 0) < 0,
> +			ERR(!i2d_CMS_bio_stream(b, cms, NULL, 0),
>  			    "%s", sig_file_name);
>  #else
> -			ERR(i2d_PKCS7_bio(b, pkcs7) < 0,
> +			ERR(!i2d_PKCS7_bio(b, pkcs7),
>  			    "%s", sig_file_name);
>  #endif
>  			BIO_free(b);
> @@ -374,9 +374,9 @@ int main(int argc, char **argv)
>  
>  	if (!raw_sig) {
>  #ifndef USE_PKCS7
> -		ERR(i2d_CMS_bio_stream(bd, cms, NULL, 0) < 0, "%s", dest_name);
> +		ERR(!i2d_CMS_bio_stream(bd, cms, NULL, 0), "%s", dest_name);
>  #else
> -		ERR(i2d_PKCS7_bio(bd, pkcs7) < 0, "%s", dest_name);
> +		ERR(!i2d_PKCS7_bio(bd, pkcs7), "%s", dest_name);
>  #endif
>  	} else {
>  		BIO *b;
> @@ -396,7 +396,7 @@ int main(int argc, char **argv)
>  	ERR(BIO_write(bd, &sig_info, sizeof(sig_info)) < 0, "%s", dest_name);
>  	ERR(BIO_write(bd, magic_number, sizeof(magic_number) - 1) < 0, "%s", dest_name);
>  
> -	ERR(BIO_free(bd) < 0, "%s", dest_name);
> +	ERR(!BIO_free(bd), "%s", dest_name);
>  
>  	/* Finally, if we're signing in place, replace the original. */
>  	if (replace_orig)
> -- 
> 2.35.1

Let's loop in directly as well David Howells and David Woodhouse as
maintainers for the certificate handling.

David and David, are you okay with the patch and if so can you pick it
up?

Regards,
Salvatore