Skip to content

Commit

Permalink
Revert "Replace CONF's internal representation with something more ty…
Browse files Browse the repository at this point in the history 
…pesafe"

This reverts commit cc932cf.
  • Loading branch information
@samuel40791765
samuel40791765 committed Nov 12, 2024
1 parent 796202b commit 6fadb50
Show file tree
Hide file tree
Showing 3 changed files with 90 additions and 93 deletions.
174 changes: 88 additions & 86 deletions crypto/conf/conf.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,51 +70,48 @@
#include "../internal.h"


struct conf_section_st {
char *name;
// values contains non-owning pointers to the values in the section.
STACK_OF(CONF_VALUE) *values;
};

static const char kDefaultSectionName[] = "default";

static uint32_t conf_section_hash(const CONF_SECTION *s) {
return OPENSSL_strhash(s->name);
}

static int conf_section_cmp(const CONF_SECTION *a, const CONF_SECTION *b) {
return strcmp(a->name, b->name);
}

static uint32_t conf_value_hash(const CONF_VALUE *v) {
const uint32_t section_hash = OPENSSL_strhash(v->section);
const uint32_t name_hash = OPENSSL_strhash(v->name);
const uint32_t section_hash = v->section ? OPENSSL_strhash(v->section) : 0;
const uint32_t name_hash = v->name ? OPENSSL_strhash(v->name) : 0;
return (section_hash << 2) ^ name_hash;
}

static int conf_value_cmp(const CONF_VALUE *a, const CONF_VALUE *b) {
int cmp = strcmp(a->section, b->section);
if (cmp != 0) {
return cmp;
int i;

if (a->section != b->section) {
i = strcmp(a->section, b->section);
if (i) {
return i;
}
}

return strcmp(a->name, b->name);
if (a->name != NULL && b->name != NULL) {
return strcmp(a->name, b->name);
} else if (a->name == b->name) {
return 0;
} else {
return (a->name == NULL) ? -1 : 1;
}
}

CONF *NCONF_new(void *method) {
CONF *conf;

if (method != NULL) {
return NULL;
}

CONF *conf = OPENSSL_malloc(sizeof(CONF));
conf = OPENSSL_malloc(sizeof(CONF));
if (conf == NULL) {
return NULL;
}

conf->sections = lh_CONF_SECTION_new(conf_section_hash, conf_section_cmp);
conf->values = lh_CONF_VALUE_new(conf_value_hash, conf_value_cmp);
if (conf->sections == NULL || conf->values == NULL) {
NCONF_free(conf);
conf->data = lh_CONF_VALUE_new(conf_value_hash, conf_value_cmp);
if (conf->data == NULL) {
OPENSSL_free(conf);
return NULL;
}

Expand All @@ -123,64 +120,69 @@ CONF *NCONF_new(void *method) {

CONF_VALUE *CONF_VALUE_new(void) { return OPENSSL_zalloc(sizeof(CONF_VALUE)); }

static void value_free(CONF_VALUE *value) {
if (value == NULL) {
return;
}
static void value_free_contents(CONF_VALUE *value) {
OPENSSL_free(value->section);
OPENSSL_free(value->name);
OPENSSL_free(value->value);
OPENSSL_free(value);
if (value->name) {
OPENSSL_free(value->name);
OPENSSL_free(value->value);
} else {
// TODO(davidben): When |value->name| is NULL, |CONF_VALUE| is actually an
// entirely different structure. This is fragile and confusing. Make a
// proper |CONF_SECTION| type that doesn't require this.
sk_CONF_VALUE_free((STACK_OF(CONF_VALUE) *)value->value);
}
}

static void section_free(CONF_SECTION *section) {
if (section == NULL) {
return;
static void value_free(CONF_VALUE *value) {
if (value != NULL) {
value_free_contents(value);
OPENSSL_free(value);
}
OPENSSL_free(section->name);
sk_CONF_VALUE_free(section->values);
OPENSSL_free(section);
}

static void value_free_arg(CONF_VALUE *value, void *arg) { value_free(value); }

static void section_free_arg(CONF_SECTION *section, void *arg) {
section_free(section);
}

void NCONF_free(CONF *conf) {
if (conf == NULL) {
if (conf == NULL || conf->data == NULL) {
return;
}

lh_CONF_SECTION_doall_arg(conf->sections, section_free_arg, NULL);
lh_CONF_SECTION_free(conf->sections);
lh_CONF_VALUE_doall_arg(conf->values, value_free_arg, NULL);
lh_CONF_VALUE_free(conf->values);
lh_CONF_VALUE_doall_arg(conf->data, value_free_arg, NULL);
lh_CONF_VALUE_free(conf->data);
OPENSSL_free(conf);
}

static CONF_SECTION *NCONF_new_section(const CONF *conf, const char *section) {
CONF_SECTION *s = OPENSSL_malloc(sizeof(CONF_SECTION));
if (!s) {
return NULL;
static CONF_VALUE *NCONF_new_section(const CONF *conf, const char *section) {
STACK_OF(CONF_VALUE) *sk = NULL;
int ok = 0;
CONF_VALUE *v = NULL, *old_value;

sk = sk_CONF_VALUE_new_null();
v = CONF_VALUE_new();
if (sk == NULL || v == NULL) {
goto err;
}
s->name = OPENSSL_strdup(section);
s->values = sk_CONF_VALUE_new_null();
if (s->name == NULL || s->values == NULL) {
v->section = OPENSSL_strdup(section);
if (v->section == NULL) {
goto err;
}

CONF_SECTION *old_section;
if (!lh_CONF_SECTION_insert(conf->sections, &old_section, s)) {
v->name = NULL;
v->value = (char *)sk;

if (!lh_CONF_VALUE_insert(conf->data, &old_value, v)) {
goto err;
}
section_free(old_section);
return s;
value_free(old_value);
ok = 1;

err:
section_free(s);
return NULL;
if (!ok) {
sk_CONF_VALUE_free(sk);
OPENSSL_free(v);
v = NULL;
}
return v;
}

static int str_copy(CONF *conf, char *section, char **pto, char *from) {
Expand Down Expand Up @@ -252,20 +254,21 @@ static int str_copy(CONF *conf, char *section, char **pto, char *from) {
return 0;
}

static CONF_SECTION *get_section(const CONF *conf, const char *section) {
CONF_SECTION template;
static CONF_VALUE *get_section(const CONF *conf, const char *section) {
CONF_VALUE template;

OPENSSL_memset(&template, 0, sizeof(template));
template.name = (char *) section;
return lh_CONF_SECTION_retrieve(conf->sections, &template);
template.section = (char *) section;
return lh_CONF_VALUE_retrieve(conf->data, &template);
}

const STACK_OF(CONF_VALUE) *NCONF_get_section(const CONF *conf,
const char *section) {
const CONF_SECTION *section_obj = get_section(conf, section);
if (section_obj == NULL) {
const CONF_VALUE *section_value = get_section(conf, section);
if (section_value == NULL) {
return NULL;
}
return section_obj->values;
return (STACK_OF(CONF_VALUE)*) section_value->value;
}

const char *NCONF_get_string(const CONF *conf, const char *section,
Expand All @@ -277,35 +280,30 @@ const char *NCONF_get_string(const CONF *conf, const char *section,
}

OPENSSL_memset(&template, 0, sizeof(template));
template.section = (char *)section;
template.name = (char *)name;
value = lh_CONF_VALUE_retrieve(conf->values, &template);
template.section = (char *) section;
template.name = (char *) name;
value = lh_CONF_VALUE_retrieve(conf->data, &template);
if (value == NULL) {
return NULL;
}
return value->value;
}

static int add_string(const CONF *conf, CONF_SECTION *section,
static int add_string(const CONF *conf, CONF_VALUE *section,
CONF_VALUE *value) {
value->section = OPENSSL_strdup(section->name);
if (value->section == NULL) {
return 0;
}
STACK_OF(CONF_VALUE) *section_stack = (STACK_OF(CONF_VALUE)*) section->value;
CONF_VALUE *old_value;

if (!sk_CONF_VALUE_push(section->values, value)) {
value->section = OPENSSL_strdup(section->section);
if (!sk_CONF_VALUE_push(section_stack, value)) {
return 0;
}

CONF_VALUE *old_value;
if (!lh_CONF_VALUE_insert(conf->values, &old_value, value)) {
// Remove |value| from |section->values|, so we do not leave a dangling
// pointer.
sk_CONF_VALUE_pop(section->values);
if (!lh_CONF_VALUE_insert(conf->data, &old_value, value)) {
return 0;
}
if (old_value != NULL) {
(void)sk_CONF_VALUE_delete_ptr(section->values, old_value);
(void)sk_CONF_VALUE_delete_ptr(section_stack, old_value);
value_free(old_value);
}

Expand Down Expand Up @@ -390,8 +388,8 @@ int NCONF_load_bio(CONF *conf, BIO *in, long *out_error_line) {
int again;
long eline = 0;
char btmp[DECIMAL_SIZE(eline) + 1];
CONF_VALUE *v = NULL;
CONF_SECTION *sv = NULL;
CONF_VALUE *v = NULL, *tv;
CONF_VALUE *sv = NULL;
char *section = NULL, *buf;
char *start, *psection, *pname;

Expand Down Expand Up @@ -542,7 +540,6 @@ int NCONF_load_bio(CONF *conf, BIO *in, long *out_error_line) {
goto err;
}

CONF_SECTION *tv;
if (strcmp(psection, section) != 0) {
if ((tv = get_section(conf, psection)) == NULL) {
tv = NCONF_new_section(conf, psection);
Expand Down Expand Up @@ -572,7 +569,12 @@ int NCONF_load_bio(CONF *conf, BIO *in, long *out_error_line) {
}
snprintf(btmp, sizeof btmp, "%ld", eline);
ERR_add_error_data(2, "line ", btmp);
value_free(v);

if (v != NULL) {
OPENSSL_free(v->name);
OPENSSL_free(v->value);
OPENSSL_free(v);
}
return 0;
}

Expand Down
3 changes: 1 addition & 2 deletions crypto/conf/conf_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,7 @@ static void ExpectConfEquals(const CONF *conf, const ConfModel &model) {

// There should not be any other values in |conf|. |conf| currently stores
// both sections and values in the same map.
EXPECT_EQ(lh_CONF_SECTION_num_items(conf->sections), model.size());
EXPECT_EQ(lh_CONF_VALUE_num_items(conf->values), total_values);
EXPECT_EQ(lh_CONF_VALUE_num_items(conf->data), total_values + model.size());
}

TEST(ConfTest, Parse) {
Expand Down
6 changes: 1 addition & 5 deletions crypto/conf/internal.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,10 @@ extern "C" {
#endif


typedef struct conf_section_st CONF_SECTION;

DEFINE_LHASH_OF(CONF_SECTION)
DEFINE_LHASH_OF(CONF_VALUE)

struct conf_st {
LHASH_OF(CONF_VALUE) *values;
LHASH_OF(CONF_SECTION) *sections;
LHASH_OF(CONF_VALUE) *data;
};

// CONF_VALUE_new returns a freshly allocated and zeroed |CONF_VALUE|.
Expand Down

0 comments on commit 6fadb50

Please sign in to comment.