The branch openssl-3.0 has been updated via 8e5ba8d0be7410fb784d5895d97dcc23d6266715 (commit) via f1c7b44ebb826ba89f5b74ae455d7e03dbe98642 (commit) from d0bfe6dc399e7071b660160d3470a050f0240013 (commit)
- Log ----------------------------------------------------------------- commit 8e5ba8d0be7410fb784d5895d97dcc23d6266715 Author: Pauli <pa...@openssl.org> Date: Tue Jan 4 10:52:52 2022 +1100 Revert "property: use a stack to efficiently convert index to string" This reverts commit e4a32f209ce6dcb380a7dc8c10a42946345ff38f. Reviewed-by: Tim Hudson <t...@openssl.org> (Merged from https://github.com/openssl/openssl/pull/17403) commit f1c7b44ebb826ba89f5b74ae455d7e03dbe98642 Author: Pauli <pa...@openssl.org> Date: Tue Jan 4 10:52:49 2022 +1100 Revert "test: add some unit tests for the property to string functions" This reverts commit e1436d54b9de5012d1716212c7329e46cf21a24a. Reviewed-by: Tim Hudson <t...@openssl.org> (Merged from https://github.com/openssl/openssl/pull/17403) ----------------------------------------------------------------------- Summary of changes: crypto/property/property_string.c | 114 +++++++++++++++++++++----------------- test/property_test.c | 61 ++++++-------------- 2 files changed, 78 insertions(+), 97 deletions(-) diff --git a/crypto/property/property_string.c b/crypto/property/property_string.c index 6c61bfbbb2..38deab5af0 100644 --- a/crypto/property/property_string.c +++ b/crypto/property/property_string.c @@ -40,8 +40,6 @@ typedef struct { PROP_TABLE *prop_values; OSSL_PROPERTY_IDX prop_name_idx; OSSL_PROPERTY_IDX prop_value_idx; - STACK_OF(OPENSSL_CSTRING) *prop_namelist; - STACK_OF(OPENSSL_CSTRING) *prop_valuelist; } PROPERTY_STRING_DATA; static unsigned long property_hash(const PROPERTY_STRING *a) @@ -80,9 +78,6 @@ static void property_string_data_free(void *vpropdata) CRYPTO_THREAD_lock_free(propdata->lock); property_table_free(&propdata->prop_names); property_table_free(&propdata->prop_values); - sk_OPENSSL_CSTRING_free(propdata->prop_namelist); - sk_OPENSSL_CSTRING_free(propdata->prop_valuelist); - propdata->prop_namelist = propdata->prop_valuelist = NULL; propdata->prop_name_idx = propdata->prop_value_idx = 0; OPENSSL_free(propdata); @@ -95,21 +90,24 @@ static void *property_string_data_new(OSSL_LIB_CTX *ctx) { return NULL; propdata->lock = CRYPTO_THREAD_lock_new(); + if (propdata->lock == NULL) + goto err; + propdata->prop_names = lh_PROPERTY_STRING_new(&property_hash, &property_cmp); + if (propdata->prop_names == NULL) + goto err; + propdata->prop_values = lh_PROPERTY_STRING_new(&property_hash, &property_cmp); - propdata->prop_namelist = sk_OPENSSL_CSTRING_new_null(); - propdata->prop_valuelist = sk_OPENSSL_CSTRING_new_null(); - if (propdata->lock == NULL - || propdata->prop_names == NULL - || propdata->prop_values == NULL - || propdata->prop_namelist == NULL - || propdata->prop_valuelist == NULL) { - property_string_data_free(propdata); - return NULL; - } + if (propdata->prop_values == NULL) + goto err; + return propdata; + +err: + property_string_data_free(propdata); + return NULL; } static const OSSL_LIB_CTX_METHOD property_string_data_method = { @@ -136,65 +134,57 @@ static PROPERTY_STRING *new_property_string(const char *s, return ps; } -static OSSL_PROPERTY_IDX ossl_property_string(OSSL_LIB_CTX *ctx, int name, - int create, const char *s) +static OSSL_PROPERTY_IDX ossl_property_string(CRYPTO_RWLOCK *lock, + PROP_TABLE *t, + OSSL_PROPERTY_IDX *pidx, + const char *s) { PROPERTY_STRING p, *ps, *ps_new; - PROP_TABLE *t; - STACK_OF(OPENSSL_CSTRING) *slist; - OSSL_PROPERTY_IDX *pidx; - PROPERTY_STRING_DATA *propdata - = ossl_lib_ctx_get_data(ctx, OSSL_LIB_CTX_PROPERTY_STRING_INDEX, - &property_string_data_method); - if (propdata == NULL) - return 0; - - t = name ? propdata->prop_names : propdata->prop_values; p.s = s; - if (!CRYPTO_THREAD_read_lock(propdata->lock)) { + if (!CRYPTO_THREAD_read_lock(lock)) { ERR_raise(ERR_LIB_CRYPTO, ERR_R_UNABLE_TO_GET_READ_LOCK); return 0; } ps = lh_PROPERTY_STRING_retrieve(t, &p); - if (ps == NULL && create) { - CRYPTO_THREAD_unlock(propdata->lock); - if (!CRYPTO_THREAD_write_lock(propdata->lock)) { + if (ps == NULL && pidx != NULL) { + CRYPTO_THREAD_unlock(lock); + if (!CRYPTO_THREAD_write_lock(lock)) { ERR_raise(ERR_LIB_CRYPTO, ERR_R_UNABLE_TO_GET_WRITE_LOCK); return 0; } - pidx = name ? &propdata->prop_name_idx : &propdata->prop_value_idx; ps = lh_PROPERTY_STRING_retrieve(t, &p); if (ps == NULL && (ps_new = new_property_string(s, pidx)) != NULL) { - slist = name ? propdata->prop_namelist : propdata->prop_valuelist; - if (sk_OPENSSL_CSTRING_push(slist, ps_new->s) <= 0) { - property_free(ps_new); - CRYPTO_THREAD_unlock(propdata->lock); - return 0; - } lh_PROPERTY_STRING_insert(t, ps_new); if (lh_PROPERTY_STRING_error(t)) { - /*- - * Undo the previous push which means also decrementing the - * index and freeing the allocated storage. - */ - sk_OPENSSL_CSTRING_pop(slist); property_free(ps_new); - --*pidx; - CRYPTO_THREAD_unlock(propdata->lock); + CRYPTO_THREAD_unlock(lock); return 0; } ps = ps_new; } } - CRYPTO_THREAD_unlock(propdata->lock); + CRYPTO_THREAD_unlock(lock); return ps != NULL ? ps->idx : 0; } +struct find_str_st { + const char *str; + OSSL_PROPERTY_IDX idx; +}; + +static void find_str_fn(PROPERTY_STRING *prop, void *vfindstr) +{ + struct find_str_st *findstr = vfindstr; + + if (prop->idx == findstr->idx) + findstr->str = prop->s; +} + static const char *ossl_property_str(int name, OSSL_LIB_CTX *ctx, OSSL_PROPERTY_IDX idx) { - const char *r; + struct find_str_st findstr; PROPERTY_STRING_DATA *propdata = ossl_lib_ctx_get_data(ctx, OSSL_LIB_CTX_PROPERTY_STRING_INDEX, &property_string_data_method); @@ -202,21 +192,33 @@ static const char *ossl_property_str(int name, OSSL_LIB_CTX *ctx, if (propdata == NULL) return NULL; + findstr.str = NULL; + findstr.idx = idx; + if (!CRYPTO_THREAD_read_lock(propdata->lock)) { ERR_raise(ERR_LIB_CRYPTO, ERR_R_UNABLE_TO_GET_READ_LOCK); return NULL; } - r = sk_OPENSSL_CSTRING_value(name ? propdata->prop_namelist - : propdata->prop_valuelist, idx - 1); + lh_PROPERTY_STRING_doall_arg(name ? propdata->prop_names + : propdata->prop_values, + find_str_fn, &findstr); CRYPTO_THREAD_unlock(propdata->lock); - return r; + return findstr.str; } OSSL_PROPERTY_IDX ossl_property_name(OSSL_LIB_CTX *ctx, const char *s, int create) { - return ossl_property_string(ctx, 1, create, s); + PROPERTY_STRING_DATA *propdata + = ossl_lib_ctx_get_data(ctx, OSSL_LIB_CTX_PROPERTY_STRING_INDEX, + &property_string_data_method); + + if (propdata == NULL) + return 0; + return ossl_property_string(propdata->lock, propdata->prop_names, + create ? &propdata->prop_name_idx : NULL, + s); } const char *ossl_property_name_str(OSSL_LIB_CTX *ctx, OSSL_PROPERTY_IDX idx) @@ -227,7 +229,15 @@ const char *ossl_property_name_str(OSSL_LIB_CTX *ctx, OSSL_PROPERTY_IDX idx) OSSL_PROPERTY_IDX ossl_property_value(OSSL_LIB_CTX *ctx, const char *s, int create) { - return ossl_property_string(ctx, 0, create, s); + PROPERTY_STRING_DATA *propdata + = ossl_lib_ctx_get_data(ctx, OSSL_LIB_CTX_PROPERTY_STRING_INDEX, + &property_string_data_method); + + if (propdata == NULL) + return 0; + return ossl_property_string(propdata->lock, propdata->prop_values, + create ? &propdata->prop_value_idx : NULL, + s); } const char *ossl_property_value_str(OSSL_LIB_CTX *ctx, OSSL_PROPERTY_IDX idx) diff --git a/test/property_test.c b/test/property_test.c index 14b891c3a0..ad44cf1513 100644 --- a/test/property_test.c +++ b/test/property_test.c @@ -50,59 +50,30 @@ static void down_ref(void *p) static int test_property_string(void) { - OSSL_LIB_CTX *ctx; - OSSL_METHOD_STORE *store = NULL; + OSSL_METHOD_STORE *store; int res = 0; OSSL_PROPERTY_IDX i, j; - /*- - * Use our own library context because we depend on ordering from a - * pristine state. - */ - if (TEST_ptr(ctx = OSSL_LIB_CTX_new()) - && TEST_ptr(store = ossl_method_store_new(ctx)) - && TEST_int_eq(ossl_property_name(ctx, "fnord", 0), 0) - && TEST_int_ne(ossl_property_name(ctx, "fnord", 1), 0) - && TEST_int_ne(ossl_property_name(ctx, "name", 1), 0) - /* Pre loaded names */ - && TEST_str_eq(ossl_property_name_str(ctx, 1), "provider") - && TEST_str_eq(ossl_property_name_str(ctx, 2), "version") - && TEST_str_eq(ossl_property_name_str(ctx, 3), "fips") - && TEST_str_eq(ossl_property_name_str(ctx, 4), "output") - && TEST_str_eq(ossl_property_name_str(ctx, 5), "input") - && TEST_str_eq(ossl_property_name_str(ctx, 6), "structure") - /* The names we added */ - && TEST_str_eq(ossl_property_name_str(ctx, 7), "fnord") - && TEST_str_eq(ossl_property_name_str(ctx, 8), "name") - /* Out of range */ - && TEST_ptr_null(ossl_property_name_str(ctx, 0)) - && TEST_ptr_null(ossl_property_name_str(ctx, 9)) + if (TEST_ptr(store = ossl_method_store_new(NULL)) + && TEST_int_eq(ossl_property_name(NULL, "fnord", 0), 0) + && TEST_int_ne(ossl_property_name(NULL, "fnord", 1), 0) + && TEST_int_ne(ossl_property_name(NULL, "name", 1), 0) /* Property value checks */ - && TEST_int_eq(ossl_property_value(ctx, "fnord", 0), 0) - && TEST_int_ne(i = ossl_property_value(ctx, "no", 0), 0) - && TEST_int_ne(j = ossl_property_value(ctx, "yes", 0), 0) + && TEST_int_eq(ossl_property_value(NULL, "fnord", 0), 0) + && TEST_int_ne(i = ossl_property_value(NULL, "no", 0), 0) + && TEST_int_ne(j = ossl_property_value(NULL, "yes", 0), 0) && TEST_int_ne(i, j) - && TEST_int_eq(ossl_property_value(ctx, "yes", 1), j) - && TEST_int_eq(ossl_property_value(ctx, "no", 1), i) - && TEST_int_ne(i = ossl_property_value(ctx, "illuminati", 1), 0) - && TEST_int_eq(j = ossl_property_value(ctx, "fnord", 1), i + 1) - && TEST_int_eq(ossl_property_value(ctx, "fnord", 1), j) - /* Pre loaded values */ - && TEST_str_eq(ossl_property_value_str(ctx, 1), "yes") - && TEST_str_eq(ossl_property_value_str(ctx, 2), "no") - /* The value we added */ - && TEST_str_eq(ossl_property_value_str(ctx, 3), "illuminati") - && TEST_str_eq(ossl_property_value_str(ctx, 4), "fnord") - /* Out of range */ - && TEST_ptr_null(ossl_property_value_str(ctx, 0)) - && TEST_ptr_null(ossl_property_value_str(ctx, 5)) + && TEST_int_eq(ossl_property_value(NULL, "yes", 1), j) + && TEST_int_eq(ossl_property_value(NULL, "no", 1), i) + && TEST_int_ne(i = ossl_property_value(NULL, "illuminati", 1), 0) + && TEST_int_eq(j = ossl_property_value(NULL, "fnord", 1), i + 1) + && TEST_int_eq(ossl_property_value(NULL, "fnord", 1), j) /* Check name and values are distinct */ - && TEST_int_eq(ossl_property_value(ctx, "cold", 0), 0) - && TEST_int_ne(ossl_property_name(ctx, "fnord", 0), - ossl_property_value(ctx, "fnord", 0))) + && TEST_int_eq(ossl_property_value(NULL, "cold", 0), 0) + && TEST_int_ne(ossl_property_name(NULL, "fnord", 0), + ossl_property_value(NULL, "fnord", 0))) res = 1; ossl_method_store_free(store); - OSSL_LIB_CTX_free(ctx); return res; }