Commit 3e47caff authored by Rich Salz's avatar Rich Salz
Browse files

ERR_ cleanup 



Remove ERR_[gs]et_implementation as they were not undocumented and
useless (the data structure was opaque).

Halve the number of lock/unlock calls in almost all ERR_
functions by letting the caller of get_hash or int_thread_set
able to lock.  Very useful when looping, such as adding errors,
or when getting the hash and immediately doing a lookup on it.

Reviewed-by: default avatarRichard Levitte <levitte@openssl.org>
parent 0223ca09

crypto/engine/eng_dyn.c

+0 −1
Original line number Diff line number Diff line
@@ -519,7 +519,6 @@ static int dynamic_load(ENGINE *e, dynamic_data_ctx *ctx) 
     * would also increase opaqueness.

     */

    fns.static_state = ENGINE_get_static_state();

    fns.err_fns = ERR_get_implementation();

    fns.ex_data_fns = CRYPTO_get_ex_data_implementation();

    CRYPTO_get_mem_functions(&fns.mem_fns.malloc_cb,

                             &fns.mem_fns.realloc_cb, &fns.mem_fns.free_cb);

crypto/err/err.c

+87 −224
Original line number Diff line number Diff line
@@ -223,109 +223,23 @@ static ERR_STRING_DATA ERR_str_reasons[] = { 
};

#endif



/* Define the predeclared (but externally opaque) "ERR_FNS" type */

struct st_ERR_FNS {

    /* Works on the "error_hash" string table */

    LHASH_OF(ERR_STRING_DATA) *(*cb_err_get) (int create);

    void (*cb_err_del) (void);

    ERR_STRING_DATA *(*cb_err_get_item) (const ERR_STRING_DATA *);

    ERR_STRING_DATA *(*cb_err_set_item) (ERR_STRING_DATA *);

    ERR_STRING_DATA *(*cb_err_del_item) (ERR_STRING_DATA *);

    /* Works on the "thread_hash" error-state table */

    LHASH_OF(ERR_STATE) *(*cb_thread_get) (int create);

    void (*cb_thread_release) (LHASH_OF(ERR_STATE) **hash);

    ERR_STATE *(*cb_thread_get_item) (const ERR_STATE *);

    ERR_STATE *(*cb_thread_set_item) (ERR_STATE *);

    void (*cb_thread_del_item) (const ERR_STATE *);

    /* Returns the next available error "library" numbers */

    int (*cb_get_next_lib) (void);

};



/* Predeclarations of the "err_defaults" functions */

static LHASH_OF(ERR_STRING_DATA) *int_err_get(int create);

static void int_err_del(void);

static LHASH_OF(ERR_STRING_DATA) *get_hash(int create, int lockit);

static ERR_STRING_DATA *int_err_get_item(const ERR_STRING_DATA *);

static ERR_STRING_DATA *int_err_set_item(ERR_STRING_DATA *);

static ERR_STRING_DATA *int_err_del_item(ERR_STRING_DATA *);

static LHASH_OF(ERR_STATE) *int_thread_get(int create);

static LHASH_OF(ERR_STATE) *int_thread_get(int create, int lockit);

static void int_thread_release(LHASH_OF(ERR_STATE) **hash);

static ERR_STATE *int_thread_get_item(const ERR_STATE *);

static ERR_STATE *int_thread_set_item(ERR_STATE *);

static void int_thread_del_item(const ERR_STATE *);

static int int_err_get_next_lib(void);

/* The static ERR_FNS table using these defaults functions */

static const ERR_FNS err_defaults = {

    int_err_get,

    int_err_del,

    int_err_get_item,

    int_err_set_item,

    int_err_del_item,

    int_thread_get,

    int_thread_release,

    int_thread_get_item,

    int_thread_set_item,

    int_thread_del_item,

    int_err_get_next_lib

};



/* The replacable table of ERR_FNS functions we use at run-time */

static const ERR_FNS *err_fns = NULL;



/* Eg. rather than using "err_get()", use "ERRFN(err_get)()". */

#define ERRFN(a) err_fns->cb_##a



/*

 * The internal state used by "err_defaults" - as such, the setting, reading,

 * creating, and deleting of this data should only be permitted via the

 * "err_defaults" functions. This way, a linked module can completely defer

 * all ERR state operation (together with requisite locking) to the

 * implementations and state in the loading application.

 * The internal state

 */

static LHASH_OF(ERR_STRING_DATA) *int_error_hash = NULL;

static LHASH_OF(ERR_STATE) *int_thread_hash = NULL;

static int int_thread_hash_references = 0;

static int int_err_library_number = ERR_LIB_USER;



/*

 * Internal function that checks whether "err_fns" is set and if not, sets it

 * to the defaults.

 */

static void err_fns_check(void)

{

    if (err_fns)

        return;



    CRYPTO_w_lock(CRYPTO_LOCK_ERR);

    if (!err_fns)

        err_fns = &err_defaults;

    CRYPTO_w_unlock(CRYPTO_LOCK_ERR);

}



/* API functions to get or set the underlying ERR functions. */



const ERR_FNS *ERR_get_implementation(void)

{

    err_fns_check();

    return err_fns;

}



int ERR_set_implementation(const ERR_FNS *fns)

{

    int ret = 0;



    CRYPTO_w_lock(CRYPTO_LOCK_ERR);

    /*

     * It's too late if 'err_fns' is non-NULL. BTW: not much point setting an

     * error is there?!

     */

    if (!err_fns) {

        err_fns = fns;

        ret = 1;

    }

    CRYPTO_w_unlock(CRYPTO_LOCK_ERR);

    return ret;

}



/*

 * These are the callbacks provided to "lh_new()" when creating the LHASH

 * tables internal to the "err_defaults" implementation.
@@ -335,8 +249,6 @@ static unsigned long get_error_values(int inc, int top, const char **file, 
                                      int *line, const char **data,

                                      int *flags);



/* The internal functions used in the "err_defaults" implementation */



static unsigned long err_string_data_hash(const ERR_STRING_DATA *a)

{

    unsigned long ret, l;
@@ -356,84 +268,39 @@ static int err_string_data_cmp(const ERR_STRING_DATA *a, 


static IMPLEMENT_LHASH_COMP_FN(err_string_data, ERR_STRING_DATA)



static LHASH_OF(ERR_STRING_DATA) *int_err_get(int create)

static LHASH_OF(ERR_STRING_DATA) *get_hash(int create, int lockit)

{

    LHASH_OF(ERR_STRING_DATA) *ret = NULL;



    if (lockit)

        CRYPTO_w_lock(CRYPTO_LOCK_ERR);

    if (!int_error_hash && create) {

        CRYPTO_push_info("int_err_get (err.c)");

        CRYPTO_push_info("get_hash (err.c)");

        int_error_hash = lh_ERR_STRING_DATA_new();

        CRYPTO_pop_info();

    }

    if (int_error_hash)

        ret = int_error_hash;

    if (lockit)

        CRYPTO_w_unlock(CRYPTO_LOCK_ERR);



    return ret;

}



static void int_err_del(void)

{

    CRYPTO_w_lock(CRYPTO_LOCK_ERR);

    if (int_error_hash) {

        lh_ERR_STRING_DATA_free(int_error_hash);

        int_error_hash = NULL;

    }

    CRYPTO_w_unlock(CRYPTO_LOCK_ERR);

}



static ERR_STRING_DATA *int_err_get_item(const ERR_STRING_DATA *d)

{

    ERR_STRING_DATA *p;

    ERR_STRING_DATA *p = NULL;

    LHASH_OF(ERR_STRING_DATA) *hash;



    err_fns_check();

    hash = ERRFN(err_get) (0);

    if (!hash)

        return NULL;



    CRYPTO_r_lock(CRYPTO_LOCK_ERR);

    hash = get_hash(0, 0);

    if (hash)

        p = lh_ERR_STRING_DATA_retrieve(hash, d);

    CRYPTO_r_unlock(CRYPTO_LOCK_ERR);



    return p;

}



static ERR_STRING_DATA *int_err_set_item(ERR_STRING_DATA *d)

{

    ERR_STRING_DATA *p;

    LHASH_OF(ERR_STRING_DATA) *hash;



    err_fns_check();

    hash = ERRFN(err_get) (1);

    if (!hash)

        return NULL;



    CRYPTO_w_lock(CRYPTO_LOCK_ERR);

    p = lh_ERR_STRING_DATA_insert(hash, d);

    CRYPTO_w_unlock(CRYPTO_LOCK_ERR);



    return p;

}



static ERR_STRING_DATA *int_err_del_item(ERR_STRING_DATA *d)

{

    ERR_STRING_DATA *p;

    LHASH_OF(ERR_STRING_DATA) *hash;



    err_fns_check();

    hash = ERRFN(err_get) (0);

    if (!hash)

        return NULL;



    CRYPTO_w_lock(CRYPTO_LOCK_ERR);

    p = lh_ERR_STRING_DATA_delete(hash, d);

    CRYPTO_w_unlock(CRYPTO_LOCK_ERR);



    return p;

}



static unsigned long err_state_hash(const ERR_STATE *a)

{

    return CRYPTO_THREADID_hash(&a->tid) * 13;
@@ -448,10 +315,11 @@ static int err_state_cmp(const ERR_STATE *a, const ERR_STATE *b) 


static IMPLEMENT_LHASH_COMP_FN(err_state, ERR_STATE)



static LHASH_OF(ERR_STATE) *int_thread_get(int create)

static LHASH_OF(ERR_STATE) *int_thread_get(int create, int lockit)

{

    LHASH_OF(ERR_STATE) *ret = NULL;



    if (lockit)

        CRYPTO_w_lock(CRYPTO_LOCK_ERR);

    if (!int_thread_hash && create) {

        CRYPTO_push_info("int_thread_get (err.c)");
@@ -462,6 +330,7 @@ static LHASH_OF(ERR_STATE) *int_thread_get(int create) 
        int_thread_hash_references++;

        ret = int_thread_hash;

    }

    if (lockit)

        CRYPTO_w_unlock(CRYPTO_LOCK_ERR);

    return ret;

}
@@ -491,76 +360,59 @@ static void int_thread_release(LHASH_OF(ERR_STATE) **hash) 


static ERR_STATE *int_thread_get_item(const ERR_STATE *d)

{

    ERR_STATE *p;

    ERR_STATE *p = NULL;

    LHASH_OF(ERR_STATE) *hash;



    err_fns_check();

    hash = ERRFN(thread_get) (0);

    if (!hash)

        return NULL;



    CRYPTO_r_lock(CRYPTO_LOCK_ERR);

    hash = int_thread_get(0, 0);

    if (hash)

        p = lh_ERR_STATE_retrieve(hash, d);

    CRYPTO_r_unlock(CRYPTO_LOCK_ERR);



    ERRFN(thread_release) (&hash);

    int_thread_release(&hash);

    return p;

}



static ERR_STATE *int_thread_set_item(ERR_STATE *d)

{

    ERR_STATE *p;

    ERR_STATE *p = NULL;

    LHASH_OF(ERR_STATE) *hash;



    err_fns_check();

    hash = ERRFN(thread_get) (1);

    if (!hash)

        return NULL;



    CRYPTO_w_lock(CRYPTO_LOCK_ERR);

    hash = int_thread_get(1, 0);

    if (hash)

        p = lh_ERR_STATE_insert(hash, d);

    CRYPTO_w_unlock(CRYPTO_LOCK_ERR);



    ERRFN(thread_release) (&hash);

    int_thread_release(&hash);

    return p;

}



static void int_thread_del_item(const ERR_STATE *d)

{

    ERR_STATE *p;

    ERR_STATE *p = NULL;

    LHASH_OF(ERR_STATE) *hash;



    err_fns_check();

    hash = ERRFN(thread_get) (0);

    if (!hash)

        return;



    CRYPTO_w_lock(CRYPTO_LOCK_ERR);

    hash = int_thread_get(0, 0);

    if (hash) {

        p = lh_ERR_STATE_delete(hash, d);

    /* make sure we don't leak memory */

        /* If there are no other references, and we just removed the

         * last item, delete the int_thread_hash */

        if (int_thread_hash_references == 1

        && int_thread_hash && lh_ERR_STATE_num_items(int_thread_hash) == 0) {

            && int_thread_hash

            && lh_ERR_STATE_num_items(int_thread_hash) == 0) {

            lh_ERR_STATE_free(int_thread_hash);

            int_thread_hash = NULL;

        }

    }

    CRYPTO_w_unlock(CRYPTO_LOCK_ERR);



    ERRFN(thread_release) (&hash);

    int_thread_release(&hash);

    if (p)

        ERR_STATE_free(p);

}



static int int_err_get_next_lib(void)

{

    int ret;



    CRYPTO_w_lock(CRYPTO_LOCK_ERR);

    ret = int_err_library_number++;

    CRYPTO_w_unlock(CRYPTO_LOCK_ERR);



    return ret;

}



#ifndef OPENSSL_NO_ERR

# define NUM_SYS_STR_REASONS 127

# define LEN_SYS_STR_REASON 32
@@ -580,8 +432,8 @@ static void build_SYS_str_reasons(void) 
{

    /* OPENSSL_malloc cannot be used here, use static storage instead */

    static char strerror_tab[NUM_SYS_STR_REASONS][LEN_SYS_STR_REASON];

    int i;

    static int init = 1;

    int i;



    CRYPTO_r_lock(CRYPTO_LOCK_ERR);

    if (!init) {
@@ -659,7 +511,6 @@ static void ERR_STATE_free(ERR_STATE *s) 


void ERR_load_ERR_strings(void)

{

    err_fns_check();

#ifndef OPENSSL_NO_ERR

    err_load_strings(0, ERR_str_libraries);

    err_load_strings(0, ERR_str_reasons);
@@ -671,12 +522,18 @@ void ERR_load_ERR_strings(void) 


static void err_load_strings(int lib, ERR_STRING_DATA *str)

{

    while (str->error) {

    LHASH_OF(ERR_STRING_DATA) *hash;



    CRYPTO_w_lock(CRYPTO_LOCK_ERR);

    hash = get_hash(1, 0);

    if (hash) {

        for (; str->error; str++) {

            if (lib)

                str->error |= ERR_PACK(lib, 0, 0);

        ERRFN(err_set_item) (str);

        str++;

            (void)lh_ERR_STRING_DATA_insert(hash, str);

        }

    }

    CRYPTO_w_unlock(CRYPTO_LOCK_ERR);

}



void ERR_load_strings(int lib, ERR_STRING_DATA *str)
@@ -687,18 +544,28 @@ void ERR_load_strings(int lib, ERR_STRING_DATA *str) 


void ERR_unload_strings(int lib, ERR_STRING_DATA *str)

{

    while (str->error) {

    LHASH_OF(ERR_STRING_DATA) *hash;



    CRYPTO_w_lock(CRYPTO_LOCK_ERR);

    hash = get_hash(0, 0);

    if (hash) {

        for (; str->error; str++) {

            if (lib)

                str->error |= ERR_PACK(lib, 0, 0);

        ERRFN(err_del_item) (str);

        str++;

            (void)lh_ERR_STRING_DATA_delete(hash, str);

        }

    }

    CRYPTO_w_unlock(CRYPTO_LOCK_ERR);

}



void ERR_free_strings(void)

{

    err_fns_check();

    ERRFN(err_del) ();

    CRYPTO_w_lock(CRYPTO_LOCK_ERR);

    if (int_error_hash) {

        lh_ERR_STRING_DATA_free(int_error_hash);

        int_error_hash = NULL;

    }

    CRYPTO_w_unlock(CRYPTO_LOCK_ERR);

}



/********************************************************/
@@ -932,20 +799,17 @@ char *ERR_error_string(unsigned long e, char *ret) 


LHASH_OF(ERR_STRING_DATA) *ERR_get_string_table(void)

{

    err_fns_check();

    return ERRFN(err_get) (0);

    return get_hash(0, 1);

}



LHASH_OF(ERR_STATE) *ERR_get_err_state_table(void)

{

    err_fns_check();

    return ERRFN(thread_get) (0);

    return int_thread_get(0, 1);

}



void ERR_release_err_state_table(LHASH_OF(ERR_STATE) **hash)

{

    err_fns_check();

    ERRFN(thread_release) (hash);

    int_thread_release(hash);

}



const char *ERR_lib_error_string(unsigned long e)
@@ -953,10 +817,9 @@ const char *ERR_lib_error_string(unsigned long e) 
    ERR_STRING_DATA d, *p;

    unsigned long l;



    err_fns_check();

    l = ERR_GET_LIB(e);

    d.error = ERR_PACK(l, 0, 0);

    p = ERRFN(err_get_item) (&d);

    p = int_err_get_item(&d);

    return ((p == NULL) ? NULL : p->string);

}
@@ -965,11 +828,10 @@ const char *ERR_func_error_string(unsigned long e) 
    ERR_STRING_DATA d, *p;

    unsigned long l, f;



    err_fns_check();

    l = ERR_GET_LIB(e);

    f = ERR_GET_FUNC(e);

    d.error = ERR_PACK(l, f, 0);

    p = ERRFN(err_get_item) (&d);

    p = int_err_get_item(&d);

    return ((p == NULL) ? NULL : p->string);

}
@@ -978,14 +840,13 @@ const char *ERR_reason_error_string(unsigned long e) 
    ERR_STRING_DATA d, *p = NULL;

    unsigned long l, r;



    err_fns_check();

    l = ERR_GET_LIB(e);

    r = ERR_GET_REASON(e);

    d.error = ERR_PACK(l, 0, r);

    p = ERRFN(err_get_item) (&d);

    p = int_err_get_item(&d);

    if (!p) {

        d.error = ERR_PACK(0, 0, r);

        p = ERRFN(err_get_item) (&d);

        p = int_err_get_item(&d);

    }

    return ((p == NULL) ? NULL : p->string);

}
@@ -998,12 +859,11 @@ void ERR_remove_thread_state(const CRYPTO_THREADID *id) 
        CRYPTO_THREADID_cpy(&tmp.tid, id);

    else

        CRYPTO_THREADID_current(&tmp.tid);

    err_fns_check();

    /*

     * thread_del_item automatically destroys the LHASH if the number of

     * items reaches zero.

     */

    ERRFN(thread_del_item) (&tmp);

    int_thread_del_item(&tmp);

}



#ifndef OPENSSL_NO_DEPRECATED
@@ -1020,10 +880,9 @@ ERR_STATE *ERR_get_state(void) 
    int i;

    CRYPTO_THREADID tid;



    err_fns_check();

    CRYPTO_THREADID_current(&tid);

    CRYPTO_THREADID_cpy(&tmp.tid, &tid);

    ret = ERRFN(thread_get_item) (&tmp);

    ret = int_thread_get_item(&tmp);



    /* ret == the error state, if NULL, make a new one */

    if (ret == NULL) {
@@ -1037,9 +896,9 @@ ERR_STATE *ERR_get_state(void) 
            ret->err_data[i] = NULL;

            ret->err_data_flags[i] = 0;

        }

        tmpp = ERRFN(thread_set_item) (ret);

        tmpp = int_thread_set_item(ret);

        /* To check if insertion failed, do a get. */

        if (ERRFN(thread_get_item) (ret) != ret) {

        if (int_thread_get_item(ret) != ret) {

            ERR_STATE_free(ret); /* could not insert it */

            return (&fallback);

        }
@@ -1055,8 +914,12 @@ ERR_STATE *ERR_get_state(void) 


int ERR_get_next_error_library(void)

{

    err_fns_check();

    return ERRFN(get_next_lib) ();

    int ret;



    CRYPTO_w_lock(CRYPTO_LOCK_ERR);

    ret = int_err_library_number++;

    CRYPTO_w_unlock(CRYPTO_LOCK_ERR);

    return ret;

}



void ERR_set_error_data(char *data, int flags)

include/openssl/engine.h

+0 −2
Original line number Diff line number Diff line
@@ -776,7 +776,6 @@ typedef struct st_dynamic_LOCK_fns { 
/* The top-level structure */

typedef struct st_dynamic_fns {

    void *static_state;

    const ERR_FNS *err_fns;

    const CRYPTO_EX_DATA_IMPL *ex_data_fns;

    dynamic_MEM_fns mem_fns;

    dynamic_LOCK_fns lock_fns;
@@ -837,7 +836,6 @@ typedef int (*dynamic_bind_engine) (ENGINE *e, const char *id, 
                CRYPTO_set_dynlock_destroy_callback(fns->lock_fns.dynlock_destroy_cb); \

                if(!CRYPTO_set_ex_data_implementation(fns->ex_data_fns)) \

                        return 0; \

                if(!ERR_set_implementation(fns->err_fns)) return 0; \

        skip_cbs: \

                if(!fn(e,id)) return 0; \

                return 1; }

include/openssl/err.h

+0 −13
Original line number Diff line number Diff line
@@ -362,19 +362,6 @@ int ERR_get_next_error_library(void); 
int ERR_set_mark(void);

int ERR_pop_to_mark(void);



/* Already defined in ossl_typ.h */

/* typedef struct st_ERR_FNS ERR_FNS; */

/*

 * An application can use this function and provide the return value to

 * loaded modules that should use the application's ERR state/functionality

 */

const ERR_FNS *ERR_get_implementation(void);

/*

 * A loaded module should call this function prior to any ERR operations

 * using the application's "ERR_FNS".

 */

int ERR_set_implementation(const ERR_FNS *fns);



#ifdef  __cplusplus

}

#endif

include/openssl/ossl_typ.h

+0 −2
Original line number Diff line number Diff line
@@ -172,8 +172,6 @@ typedef struct store_method_st STORE_METHOD; 
typedef struct ui_st UI;

typedef struct ui_method_st UI_METHOD;



typedef struct st_ERR_FNS ERR_FNS;



typedef struct engine_st ENGINE;

typedef struct ssl_st SSL;

typedef struct ssl_ctx_st SSL_CTX;