Skip to content

Add script to convert XXXerr() to XXXerror #9441

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
richsalz wants to merge 4 commits into from
Closed

Add script to convert XXXerr() to XXXerror #9441

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fa0ecc1
Add script convert XXerr to ERR_raise
richsalz Jul 24, 2019
8bedbbc
Deprecate XXXerr() macros
richsalz Jul 30, 2019
9c2f887
Add merge-err-lines script
richsalz Aug 12, 2019
2e85061
Avoid ?: construct in XXXerr calls
richsalz Aug 16, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions crypto/evp/evp_lib.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,10 @@ int EVP_CIPHER_param_to_asn1(EVP_CIPHER_CTX *c, ASN1_TYPE *type)
}
} else
ret = -1;
if (ret <= 0)
EVPerr(EVP_F_EVP_CIPHER_PARAM_TO_ASN1, ret == -2 ?
ASN1_R_UNSUPPORTED_CIPHER :
EVP_R_CIPHER_PARAMETER_ERROR);
if (ret == -2)
EVPerr(EVP_F_EVP_CIPHER_PARAM_TO_ASN1, ASN1_R_UNSUPPORTED_CIPHER);
else if (ret <= 0)
EVPerr(EVP_F_EVP_CIPHER_PARAM_TO_ASN1, EVP_R_CIPHER_PARAMETER_ERROR);
Copy link
Member

@beldmit beldmit Sep 19, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for being out of context, but is it OK that here we leave EVPerr(), and in https://github.com/openssl/openssl/pull/9441/files#diff-9f773e58c4cc40339f694534e55f6cb3 we replace EVPerr with ERR_raise()?

Copy link
Member

@levitte levitte Sep 19, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these are changes are made to make life easier with util/merge-err-lines, as that only handles this kind of split:

    BLARGerr(BLARG_F_FOO,
             BLARG_R_BAR);

Copy link
Member

@beldmit beldmit Sep 19, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it.

if (ret < -1)
ret = -1;
return ret;
Expand Down Expand Up @@ -106,10 +106,10 @@ int EVP_CIPHER_asn1_to_param(EVP_CIPHER_CTX *c, ASN1_TYPE *type)
}
} else
ret = -1;
if (ret <= 0)
EVPerr(EVP_F_EVP_CIPHER_ASN1_TO_PARAM, ret == -2 ?
EVP_R_UNSUPPORTED_CIPHER :
EVP_R_CIPHER_PARAMETER_ERROR);
if (ret == -2)
EVPerr(EVP_F_EVP_CIPHER_ASN1_TO_PARAM, EVP_R_UNSUPPORTED_CIPHER);
else if (ret <= 0)
EVPerr(EVP_F_EVP_CIPHER_ASN1_TO_PARAM, EVP_R_CIPHER_PARAMETER_ERROR);
Copy link
Member

@beldmit beldmit Sep 19, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar question.

Copy link
Member

@levitte levitte Sep 19, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same answer

if (ret < -1)
ret = -1;
return ret;
Expand Down
4 changes: 2 additions & 2 deletions crypto/evp/evp_locl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,15 +233,15 @@ OSSL_PARAM *evp_pkey_to_param(EVP_PKEY *pkey, size_t *sz);
size_t pksize = (size_t)EVP_PKEY_size(ctx->pkey); \
\
if (pksize == 0) { \
EVPerr(err, EVP_R_INVALID_KEY); /*ckerr_ignore*/ \
ERR_raise(ERR_LIB_EVP, EVP_R_INVALID_KEY); /*ckerr_ignore*/ \
return 0; \
} \
if (arg == NULL) { \
*arglen = pksize; \
return 1; \
} \
if (*arglen < pksize) { \
EVPerr(err, EVP_R_BUFFER_TOO_SMALL); /*ckerr_ignore*/ \
ERR_raise(ERR_LIB_EVP, EVP_R_BUFFER_TOO_SMALL); /*ckerr_ignore*/ \
return 0; \
} \
}
Expand Down
7 changes: 4 additions & 3 deletions crypto/rand/drbg_lib.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -541,9 +541,10 @@ int RAND_DRBG_instantiate(RAND_DRBG *drbg,
}

if (drbg->state != DRBG_UNINITIALISED) {
RANDerr(RAND_F_RAND_DRBG_INSTANTIATE,
drbg->state == DRBG_ERROR ? RAND_R_IN_ERROR_STATE
: RAND_R_ALREADY_INSTANTIATED);
if (drbg->state == DRBG_ERROR)
RANDerr(RAND_F_RAND_DRBG_INSTANTIATE, RAND_R_IN_ERROR_STATE);
else
RANDerr(RAND_F_RAND_DRBG_INSTANTIATE, RAND_R_ALREADY_INSTANTIATED);
goto end;
}

Expand Down
83 changes: 42 additions & 41 deletions include/openssl/err.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,48 +108,49 @@ struct err_state_st {

# define ERR_LIB_USER 128

# if ! OPENSSL_API_3
# define SYSerr(f,r) ERR_raise(ERR_LIB_SYS,(r))
# if 1 || !OPENSSL_API_3
# define ASN1err(f, r) ERR_raise_data(ERR_LIB_ASN1, (r), NULL)
# define ASYNCerr(f, r) ERR_raise_data(ERR_LIB_ASYNC, (r), NULL)
# define BIOerr(f, r) ERR_raise_data(ERR_LIB_BIO, (r), NULL)
# define BNerr(f, r) ERR_raise_data(ERR_LIB_BN, (r), NULL)
# define BUFerr(f, r) ERR_raise_data(ERR_LIB_BUF, (r), NULL)
# define CMPerr(f, r) ERR_raise_data(ERR_LIB_CMP, (r), NULL)
# define CMSerr(f, r) ERR_raise_data(ERR_LIB_CMS, (r), NULL)
# define COMPerr(f, r) ERR_raise_data(ERR_LIB_COMP, (r), NULL)
# define CONFerr(f, r) ERR_raise_data(ERR_LIB_CONF, (r), NULL)
# define CRMFerr(f, r) ERR_raise_data(ERR_LIB_CRMF, (r), NULL)
# define CRYPTOerr(f, r) ERR_raise_data(ERR_LIB_CRYPTO, (r), NULL)
# define CTerr(f, r) ERR_raise_data(ERR_LIB_CT, (r), NULL)
# define DHerr(f, r) ERR_raise_data(ERR_LIB_DH, (r), NULL)
# define DSAerr(f, r) ERR_raise_data(ERR_LIB_DSA, (r), NULL)
# define DSOerr(f, r) ERR_raise_data(ERR_LIB_DSO, (r), NULL)
# define ECDHerr(f, r) ERR_raise_data(ERR_LIB_ECDH, (r), NULL)
# define ECDSAerr(f, r) ERR_raise_data(ERR_LIB_ECDSA, (r), NULL)
# define ECerr(f, r) ERR_raise_data(ERR_LIB_EC, (r), NULL)
# define ENGINEerr(f, r) ERR_raise_data(ERR_LIB_ENGINE, (r), NULL)
# define ESSerr(f, r) ERR_raise_data(ERR_LIB_ESS, (r), NULL)
# define EVPerr(f, r) ERR_raise_data(ERR_LIB_EVP, (r), NULL)
# define FIPSerr(f, r) ERR_raise_data(ERR_LIB_FIPS, (r), NULL)
# define HMACerr(f, r) ERR_raise_data(ERR_LIB_HMAC, (r), NULL)
# define KDFerr(f, r) ERR_raise_data(ERR_LIB_KDF, (r), NULL)
# define OBJerr(f, r) ERR_raise_data(ERR_LIB_OBJ, (r), NULL)
# define OCSPerr(f, r) ERR_raise_data(ERR_LIB_OCSP, (r), NULL)
# define OSSL_STOREerr(f, r) ERR_raise_data(ERR_LIB_OSSL_STORE, (r), NULL)
# define PEMerr(f, r) ERR_raise_data(ERR_LIB_PEM, (r), NULL)
# define PKCS12err(f, r) ERR_raise_data(ERR_LIB_PKCS12, (r), NULL)
# define PKCS7err(f, r) ERR_raise_data(ERR_LIB_PKCS7, (r), NULL)
# define PROPerr(f, r) ERR_raise_data(ERR_LIB_PROP, (r), NULL)
# define PROVerr(f, r) ERR_raise_data(ERR_LIB_PROV, (r), NULL)
# define RANDerr(f, r) ERR_raise_data(ERR_LIB_RAND, (r), NULL)
# define RSAerr(f, r) ERR_raise_data(ERR_LIB_RSA, (r), NULL)
# define SM2err(f, r) ERR_raise_data(ERR_LIB_SM2, (r), NULL)
# define SSLerr(f, r) ERR_raise_data(ERR_LIB_SSL, (r), NULL)
# define SYSerr(f, r) ERR_raise_data(ERR_LIB_SYS, (r), NULL)
# define TSerr(f, r) ERR_raise_data(ERR_LIB_TS, (r), NULL)
# define UIerr(f, r) ERR_raise_data(ERR_LIB_UI, (r), NULL)
# define X509V3err(f, r) ERR_raise_data(ERR_LIB_X509V3, (r), NULL)
# define X509err(f, r) ERR_raise_data(ERR_LIB_X509, (r), NULL)
Copy link
Member

@levitte levitte Aug 17, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems a little unnecessary to make them into ERR_raise_data...

Copy link
Contributor Author

@richsalz richsalz Aug 17, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, it's hidden behind a macro. If I leave it as ERR_raise that will then get turned into ERR_raise_data so I figured it was more useful to developers to not have to chase down multiple define's. But if you want it changed, okay.

Copy link
Contributor Author

@richsalz richsalz Aug 17, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(By which I mean, be expilcit and say "please change it" :)

Copy link
Member

@levitte levitte Aug 18, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I only made a comment, not a change request. I'll try to remember to be explicit about that)

Copy link
Contributor Author

@richsalz richsalz Aug 18, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not a big deal; I think we sometimes forget that discussion is one of the most important things about a PR. :)

# endif
# define BNerr(f,r) ERR_raise(ERR_LIB_BN,(r))
# define RSAerr(f,r) ERR_raise(ERR_LIB_RSA,(r))
# define DHerr(f,r) ERR_raise(ERR_LIB_DH,(r))
# define EVPerr(f,r) ERR_raise(ERR_LIB_EVP,(r))
# define BUFerr(f,r) ERR_raise(ERR_LIB_BUF,(r))
# define OBJerr(f,r) ERR_raise(ERR_LIB_OBJ,(r))
# define PEMerr(f,r) ERR_raise(ERR_LIB_PEM,(r))
# define DSAerr(f,r) ERR_raise(ERR_LIB_DSA,(r))
# define X509err(f,r) ERR_raise(ERR_LIB_X509,(r))
# define ASN1err(f,r) ERR_raise(ERR_LIB_ASN1,(r))
# define CONFerr(f,r) ERR_raise(ERR_LIB_CONF,(r))
# define CRYPTOerr(f,r) ERR_raise(ERR_LIB_CRYPTO,(r))
# define ECerr(f,r) ERR_raise(ERR_LIB_EC,(r))
# define SSLerr(f,r) ERR_raise(ERR_LIB_SSL,(r))
# define BIOerr(f,r) ERR_raise(ERR_LIB_BIO,(r))
# define PKCS7err(f,r) ERR_raise(ERR_LIB_PKCS7,(r))
# define X509V3err(f,r) ERR_raise(ERR_LIB_X509V3,(r))
# define PKCS12err(f,r) ERR_raise(ERR_LIB_PKCS12,(r))
# define RANDerr(f,r) ERR_raise(ERR_LIB_RAND,(r))
# define DSOerr(f,r) ERR_raise(ERR_LIB_DSO,(r))
# define ENGINEerr(f,r) ERR_raise(ERR_LIB_ENGINE,(r))
# define OCSPerr(f,r) ERR_raise(ERR_LIB_OCSP,(r))
# define UIerr(f,r) ERR_raise(ERR_LIB_UI,(r))
# define COMPerr(f,r) ERR_raise(ERR_LIB_COMP,(r))
# define ECDSAerr(f,r) ERR_raise(ERR_LIB_ECDSA,(r))
# define ECDHerr(f,r) ERR_raise(ERR_LIB_ECDH,(r))
# define OSSL_STOREerr(f,r) ERR_raise(ERR_LIB_OSSL_STORE,(r))
# define FIPSerr(f,r) ERR_raise(ERR_LIB_FIPS,(r))
# define CMSerr(f,r) ERR_raise(ERR_LIB_CMS,(r))
# define CRMFerr(f,r) ERR_raise(ERR_LIB_CRMF,(r))
# define CMPerr(f,r) ERR_raise(ERR_LIB_CMP,(r))
# define TSerr(f,r) ERR_raise(ERR_LIB_TS,(r))
# define HMACerr(f,r) ERR_raise(ERR_LIB_HMAC,(r))
# define CTerr(f,r) ERR_raise(ERR_LIB_CT,(r))
# define ASYNCerr(f,r) ERR_raise(ERR_LIB_ASYNC,(r))
# define SM2err(f,r) ERR_raise(ERR_LIB_SM2,(r))
# define ESSerr(f,r) ERR_raise(ERR_LIB_ESS,(r))
# define PROPerr(f,r) ERR_raise(ERR_LIB_PROP,(r))
# define PROVerr(f,r) ERR_raise(ERR_LIB_PROV,(r))

# define ERR_PACK(l,f,r) ( \
(((unsigned int)(l) & 0x0FF) << 24L) | \
Expand Down
62 changes: 62 additions & 0 deletions util/err-to-raise
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
#! /usr/bin/env perl
# Copyright 2019 The OpenSSL Project Authors. All Rights Reserved.
#
# Licensed under the Apache License 2.0 (the "License"). You may not use
# this file except in compliance with the License. You can obtain a copy
# in the file LICENSE in the source distribution or at
# https://www.openssl.org/source/license.html

# Run this program like this:
# perl -pi util/err-to-error files...
# or
# git ls-files | grep '\.c$' | xargs perl -pi util/err-to-raise
# Consider running util/merge-err-lines first, to catch most (all?) of the
# cases where the XXXerr() call is split into two lines.

# Also, what to do about the engines files? This includes:
# AFALGerr, CAPIerr, DASYNC, OSSLTEST
# There are about 70 such lines.

use strict;
use warnings;

s/ASN1err\(\w+, *(\w+)\)/ERR_raise(ERR_LIB_ASN1, $1)/;
s/([^D])ASYNCerr\(\w+, *(\w+)\)/$1ERR_raise(ERR_LIB_ASYNC, $2)/;
s/BIOerr\(\w+, *(\w+)\)/ERR_raise(ERR_LIB_BIO, $1)/;
s/BNerr\(\w+, *(\w+)\)/ERR_raise(ERR_LIB_BN, $1)/;
s/BUFerr\(\w+, *(\w+)\)/ERR_raise(ERR_LIB_BUF, $1)/;
s/CMPerr\(\w+, *(\w+)\)/ERR_raise(ERR_LIB_CMP, $1)/;
s/CMSerr\(\w+, *(\w+)\)/ERR_raise(ERR_LIB_CMS, $1)/;
s/COMPerr\(\w+, *(\w+)\)/ERR_raise(ERR_LIB_COMP, $1)/;
s/CONFerr\(\w+, *(\w+)\)/ERR_raise(ERR_LIB_CONF, $1)/;
s/CRMFerr\(\w+, *(\w+)\)/ERR_raise(ERR_LIB_CRMF, $1)/;
s/CRYPTOerr\(\w+, *(\w+)\)/ERR_raise(ERR_LIB_CRYPTO, $1)/;
s/CTerr\(\w+, *(\w+)\)/ERR_raise(ERR_LIB_CT, $1)/;
s/DHerr\(\w+, *(\w+)\)/ERR_raise(ERR_LIB_DH, $1)/;
s/DSAerr\(\w+, *(\w+)\)/ERR_raise(ERR_LIB_DSA, $1)/;
s/DSOerr\(\w+, *(\w+)\)/ERR_raise(ERR_LIB_DSO, $1)/;
s/ECDHerr\(\w+, *(\w+)\)/ERR_raise(ERR_LIB_ECDH, $1)/;
s/ECDSAerr\(\w+, *(\w+)\)/ERR_raise(ERR_LIB_ECDSA, $1)/;
s/ECerr\(\w+, *(\w+)\)/ERR_raise(ERR_LIB_EC, $1)/;
s/ENGINEerr\(\w+, *(\w+)\)/ERR_raise(ERR_LIB_ENGINE, $1)/;
s/ESSerr\(\w+, *(\w+)\)/ERR_raise(ERR_LIB_ESS, $1)/;
s/EVPerr\(\w+, *(\w+)\)/ERR_raise(ERR_LIB_EVP, $1)/;
s/FIPSerr\(\w+, *(\w+)\)/ERR_raise(ERR_LIB_FIPS, $1)/;
s/KDFerr\(\w+, *(\w+)\)/ERR_raise(ERR_LIB_KDF, $1)/;
s/OBJerr\(\w+, *(\w+)\)/ERR_raise(ERR_LIB_OBJ, $1)/;
s/OCSPerr\(\w+, *(\w+)\)/ERR_raise(ERR_LIB_OCSP, $1)/;
s/OSSL_STOREerr\(\w+, *(\w+)\)/ERR_raise(ERR_LIB_OSSL_STORE, $1)/;
s/PEMerr\(\w+, *(\w+)\)/ERR_raise(ERR_LIB_PEM, $1)/;
s/PKCS12err\(\w+, *(\w+)\)/ERR_raise(ERR_LIB_PKCS12, $1)/;
s/PKCS7err\(\w+, *(\w+)\)/ERR_raise(ERR_LIB_PKCS7, $1)/;
s/PROPerr\(\w+, *(\w+)\)/ERR_raise(ERR_LIB_PROP, $1)/;
s/PROVerr\(\w+, *(\w+)\)/ERR_raise(ERR_LIB_PROV, $1)/;
s/RANDerr\(\w+, *(\w+)\)/ERR_raise(ERR_LIB_RAND, $1)/;
s/RSAerr\(\w+, *(\w+)\)/ERR_raise(ERR_LIB_RSA, $1)/;
s/SM2err\(\w+, *(\w+)\)/ERR_raise(ERR_LIB_SM2, $1)/;
s/SSLerr\(\w+, *(\w+)\)/ERR_raise(ERR_LIB_SSL, $1)/;
s/SYSerr\(\w+, *(\w+)\)/ERR_raise(ERR_LIB_SYS, $1)/;
s/TSerr\(\w+, *(\w+)\)/ERR_raise(ERR_LIB_TS, $1)/;
s/UIerr\(\w+, *(\w+)\)/ERR_raise(ERR_LIB_UI, $1)/;
s/X509V3err\(\w+, *(\w+)\)/ERR_raise(ERR_LIB_X509V3, $1)/;
s/X509err\(\w+, *(\w+)\)/ERR_raise(ERR_LIB_X509, $1)/;
29 changes: 29 additions & 0 deletions util/merge-err-lines
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
#! /usr/bin/env perl
# Copyright 2019 The OpenSSL Project Authors. All Rights Reserved.
#
# Licensed under the Apache License 2.0 (the "License"). You may not use
# this file except in compliance with the License. You can obtain a copy
# in the file LICENSE in the source distribution or at
# https://www.openssl.org/source/license.html

# Sometimes calls to XXXerr() are split into two lines, because the define'd
# names are very long. This script looks for those lines and merges them.
# It should be run before the "err-to-raise" script.

# Run this program like this:
# perl -pi util/merge-err-lines files...
# or
# git grep -l '[A-Z0-9]err([^)]*$' | xargs perl -pi util/merge-err-lines

use strict;
use warnings;

# Look for "{whitespace}XXXerr(no-close-paren{WHITESPACE}" lines
if ( /^ *[_A-Z0-9]+err\([^)]+ *$/ ) {
my $copy = $_;
chop($copy);
$copy =~ s/ +$//;
my $next = <>;
$next =~ s/^ +//;
$_ = $copy . ' ' . $next;
}