From 8a9af06c905c3576991d9c1d15fa23c1aaf09998 Mon Sep 17 00:00:00 2001 From: Gina Peter Bnayard Date: Sun, 11 Aug 2024 02:55:08 +0200 Subject: [PATCH 1/8] ext/dba/inifile: Add tests handling group keys Turns out, nothing works --- ext/dba/tests/dba_inifile_group_keys.phpt | 146 ++++++++++++++++++++++ 1 file changed, 146 insertions(+) create mode 100644 ext/dba/tests/dba_inifile_group_keys.phpt diff --git a/ext/dba/tests/dba_inifile_group_keys.phpt b/ext/dba/tests/dba_inifile_group_keys.phpt new file mode 100644 index 0000000000000..eb305275749ea --- /dev/null +++ b/ext/dba/tests/dba_inifile_group_keys.phpt @@ -0,0 +1,146 @@ +--TEST-- +DBA check behaviour of array keys (inifile version) +--EXTENSIONS-- +dba +--SKIPIF-- + +--FILE-- + +--CLEAN-- + +--EXPECT-- +Checks on an empty database: +bool(false) +bool(true) +bool(true) +string(12) "Normal group" +bool(true) +Insert various group values: +bool(true) +bool(true) +bool(true) +bool(true) +bool(true) +Check INI file content: +bool(true) +Insert an existing value in group1, this should fail +bool(true) +string(12) "Group1Value2" +Check INI file content: +bool(true) +Insert a new value in group1 +bool(true) +string(12) "Group1Value4" +Check INI file content: +bool(true) +Replace value of ['group1', 'name3'] with 'Group1Value3replaced' +bool(true) +string(12) "Group1Value3" +Check INI file content: +bool(true) From 3ceb7caf874e6e6dabc41d9877f0d2e496cc8637 Mon Sep 17 00:00:00 2001 From: Gina Peter Bnayard Date: Fri, 9 Aug 2024 19:30:41 +0200 Subject: [PATCH 2/8] ext/dba/inifile: Do some refactoring This is an attempt to remove some PHPism/ZENDism from the library --- ext/dba/libinifile/inifile.c | 224 ++++++++++++++++------------------- ext/dba/libinifile/inifile.h | 13 +- 2 files changed, 107 insertions(+), 130 deletions(-) diff --git a/ext/dba/libinifile/inifile.c b/ext/dba/libinifile/inifile.c index c5467396d4bfe..f27b6738becc8 100644 --- a/ext/dba/libinifile/inifile.c +++ b/ext/dba/libinifile/inifile.c @@ -37,14 +37,11 @@ * ret = 1 key already exists - nothing done */ -/* {{{ inifile_version */ const char *inifile_version(void) { return "1.0, $Id$"; } -/* }}} */ -/* {{{ inifile_free_key */ void inifile_key_free(key_type *key) { if (key->group) { @@ -55,9 +52,7 @@ void inifile_key_free(key_type *key) } memset(key, 0, sizeof(key_type)); } -/* }}} */ -/* {{{ inifile_free_val */ void inifile_val_free(val_type *val) { if (val->value) { @@ -65,19 +60,16 @@ void inifile_val_free(val_type *val) } memset(val, 0, sizeof(val_type)); } -/* }}} */ -/* {{{ inifile_free_val */ void inifile_line_free(line_type *ln) { inifile_key_free(&ln->key); inifile_val_free(&ln->val); ln->pos = 0; } -/* }}} */ /* {{{ inifile_alloc */ -inifile * inifile_alloc(php_stream *fp, int readonly, int persistent) +inifile * inifile_alloc(php_stream *fp, bool readonly, bool persistent) { inifile *dba; @@ -96,8 +88,7 @@ inifile * inifile_alloc(php_stream *fp, int readonly, int persistent) } /* }}} */ -/* {{{ inifile_free */ -void inifile_free(inifile *dba, int persistent) +void inifile_free(inifile *dba, bool persistent) { if (dba) { inifile_line_free(&dba->curr); @@ -105,7 +96,6 @@ void inifile_free(inifile *dba, int persistent) pefree(dba, persistent); } } -/* }}} */ /* {{{ inifile_key_split */ key_type inifile_key_split(const char *group_name) @@ -216,23 +206,24 @@ static int inifile_read(inifile *dba, line_type *ln) { } /* }}} */ -/* {{{ inifile_key_cmp */ -/* 0 = EQUAL - * 1 = GROUP-EQUAL,NAME-DIFFERENT - * 2 = DIFFERENT - */ -static int inifile_key_cmp(const key_type *k1, const key_type *k2) +enum inifile_key_cmp_status { + EQUAL, + GROUP_EQUAL_NAME_DIFFERENT, + DIFFERENT +}; + +static enum inifile_key_cmp_status inifile_key_cmp(const key_type *k1, const key_type *k2) { assert(k1->group && k1->name && k2->group && k2->name); if (!strcasecmp(k1->group, k2->group)) { if (!strcasecmp(k1->name, k2->name)) { - return 0; + return EQUAL; } else { - return 1; + return GROUP_EQUAL_NAME_DIFFERENT; } } else { - return 2; + return DIFFERENT; } } /* }}} */ @@ -241,7 +232,6 @@ static int inifile_key_cmp(const key_type *k1, const key_type *k2) val_type inifile_fetch(inifile *dba, const key_type *key, int skip) { line_type ln = {{NULL,NULL},{NULL},0}; val_type val; - int res, grp_eq = 0; if (skip == -1 && dba->next.key.group && dba->next.key.name && !inifile_key_cmp(&dba->next.key, key)) { /* we got position already from last fetch */ @@ -256,8 +246,10 @@ val_type inifile_fetch(inifile *dba, const key_type *key, int skip) { if (skip == -1) { skip = 0; } - while(inifile_read(dba, &ln)) { - if (!(res=inifile_key_cmp(&ln.key, key))) { + bool grp_eq = false; + while (inifile_read(dba, &ln)) { + enum inifile_key_cmp_status cmp_result = inifile_key_cmp(&ln.key, key); + if (cmp_result == EQUAL) { if (!skip) { val.value = estrdup(ln.val.value ? ln.val.value : ""); /* allow faster access by updating key read into next */ @@ -267,8 +259,8 @@ val_type inifile_fetch(inifile *dba, const key_type *key, int skip) { return val; } skip--; - } else if (res == 1) { - grp_eq = 1; + } else if (cmp_result == GROUP_EQUAL_NAME_DIFFERENT) { + grp_eq = true; } else if (grp_eq) { /* we are leaving group now: that means we cannot find the key */ break; @@ -281,7 +273,7 @@ val_type inifile_fetch(inifile *dba, const key_type *key, int skip) { /* }}} */ /* {{{ inifile_firstkey */ -int inifile_firstkey(inifile *dba) { +bool inifile_firstkey(inifile *dba) { inifile_line_free(&dba->curr); dba->curr.pos = 0; return inifile_nextkey(dba); @@ -289,7 +281,7 @@ int inifile_firstkey(inifile *dba) { /* }}} */ /* {{{ inifile_nextkey */ -int inifile_nextkey(inifile *dba) { +bool inifile_nextkey(inifile *dba) { line_type ln = {{NULL,NULL},{NULL},0}; /*inifile_line_free(&dba->next); ??? */ @@ -302,19 +294,17 @@ int inifile_nextkey(inifile *dba) { } /* }}} */ -/* {{{ inifile_truncate */ -static int inifile_truncate(inifile *dba, size_t size) +static bool inifile_truncate(inifile *dba, size_t size) { - int res; + int stream_op_status = php_stream_truncate_set_size(dba->fp, size); - if ((res=php_stream_truncate_set_size(dba->fp, size)) != 0) { - php_error_docref(NULL, E_WARNING, "Error in ftruncate: %d", res); - return FAILURE; + if (stream_op_status != 0) { + php_error_docref(NULL, E_WARNING, "Error in ftruncate: %d", stream_op_status); + return false; } php_stream_seek(dba->fp, size, SEEK_SET); - return SUCCESS; + return true; } -/* }}} */ /* {{{ inifile_find_group * if found pos_grp_start points to "[group_name]" @@ -329,12 +319,10 @@ static int inifile_find_group(inifile *dba, const key_type *key, size_t *pos_grp inifile_line_free(&dba->next); if (key->group && strlen(key->group)) { - int res; line_type ln = {{NULL,NULL},{NULL},0}; - res = 1; - while(inifile_read(dba, &ln)) { - if ((res=inifile_key_cmp(&ln.key, key)) < 2) { + while (inifile_read(dba, &ln)) { + if (inifile_key_cmp(&ln.key, key) != DIFFERENT) { ret = SUCCESS; break; } @@ -356,52 +344,50 @@ static int inifile_find_group(inifile *dba, const key_type *key, size_t *pos_grp * only valid after a call to inifile_find_group * if any next group is found pos_grp_start points to "[group_name]" or whitespace before that */ -static int inifile_next_group(inifile *dba, const key_type *key, size_t *pos_grp_start) +static bool inifile_next_group(inifile *dba, const key_type *key, size_t *pos_grp_start) { - int ret = FAILURE; + bool has_next_group = false; line_type ln = {{NULL,NULL},{NULL},0}; *pos_grp_start = php_stream_tell(dba->fp); ln.key.group = estrdup(key->group); - while(inifile_read(dba, &ln)) { - if (inifile_key_cmp(&ln.key, key) == 2) { - ret = SUCCESS; + while (inifile_read(dba, &ln)) { + if (inifile_key_cmp(&ln.key, key) == DIFFERENT) { + has_next_group = true; break; } *pos_grp_start = php_stream_tell(dba->fp); } inifile_line_free(&ln); - return ret; + return has_next_group; } /* }}} */ -/* {{{ inifile_copy_to */ -static int inifile_copy_to(inifile *dba, size_t pos_start, size_t pos_end, inifile **ini_copy) +static bool inifile_copy_to(inifile *dba, size_t pos_start, size_t pos_end, inifile **ini_copy) { php_stream *fp; if (pos_start == pos_end) { *ini_copy = NULL; - return SUCCESS; + return false; } if ((fp = php_stream_temp_create(0, 64 * 1024)) == NULL) { php_error_docref(NULL, E_WARNING, "Could not create temporary stream"); *ini_copy = NULL; - return FAILURE; + return false; } if ((*ini_copy = inifile_alloc(fp, 1, 0)) == NULL) { /* writes error */ - return FAILURE; + return false; } php_stream_seek(dba->fp, pos_start, SEEK_SET); if (SUCCESS != php_stream_copy_to_stream_ex(dba->fp, fp, pos_end - pos_start, NULL)) { php_error_docref(NULL, E_WARNING, "Could not copy group [%zu - %zu] to temporary stream", pos_start, pos_end); - return FAILURE; + return false; } - return SUCCESS; + return true; } -/* }}} */ /* {{{ inifile_filter * copy from to dba while ignoring key name (group must equal) @@ -414,11 +400,11 @@ static int inifile_filter(inifile *dba, inifile *from, const key_type *key, bool php_stream_seek(from->fp, 0, SEEK_SET); php_stream_seek(dba->fp, 0, SEEK_END); - while(inifile_read(from, &ln)) { - switch(inifile_key_cmp(&ln.key, key)) { - case 0: + while (inifile_read(from, &ln)) { + switch (inifile_key_cmp(&ln.key, key)) { + case EQUAL: if (found) { - *found = (bool) 1; + *found = true; } pos_curr = php_stream_tell(from->fp); if (pos_start != pos_next) { @@ -431,10 +417,10 @@ static int inifile_filter(inifile *dba, inifile *from, const key_type *key, bool } pos_next = pos_start = pos_curr; break; - case 1: + case GROUP_EQUAL_NAME_DIFFERENT: pos_next = php_stream_tell(from->fp); break; - case 2: + case DIFFERENT: /* the function is meant to process only entries from same group */ assert(0); break; @@ -453,12 +439,11 @@ static int inifile_filter(inifile *dba, inifile *from, const key_type *key, bool /* }}} */ /* {{{ inifile_delete_replace_append */ -static int inifile_delete_replace_append(inifile *dba, const key_type *key, const val_type *value, int append, bool *found) +static int inifile_delete_replace_append(inifile *dba, const key_type *key, const val_type *value, bool is_append, bool *found) { - size_t pos_grp_start=0, pos_grp_next; + size_t pos_grp_start = 0, pos_grp_next; inifile *ini_tmp = NULL; php_stream *fp_tmp = NULL; - int ret; /* 1) Search group start * 2) Search next group @@ -471,78 +456,77 @@ static int inifile_delete_replace_append(inifile *dba, const key_type *key, cons * 8) Append temporary stream */ - assert(!append || (key->name && value)); /* missuse */ + assert(!is_append || (key->name && value)); /* missuse */ /* 1 - 3 */ inifile_find_group(dba, key, &pos_grp_start); inifile_next_group(dba, key, &pos_grp_next); - if (append) { - ret = SUCCESS; - } else { - ret = inifile_copy_to(dba, pos_grp_start, pos_grp_next, &ini_tmp); + if (!is_append && !inifile_copy_to(dba, pos_grp_start, pos_grp_next, &ini_tmp)) { + goto end; } /* 4 */ - if (ret == SUCCESS) { - fp_tmp = php_stream_temp_create(0, 64 * 1024); - if (!fp_tmp) { - php_error_docref(NULL, E_WARNING, "Could not create temporary stream"); - ret = FAILURE; - } else { - php_stream_seek(dba->fp, 0, SEEK_END); - if (pos_grp_next != (size_t)php_stream_tell(dba->fp)) { - php_stream_seek(dba->fp, pos_grp_next, SEEK_SET); - if (SUCCESS != php_stream_copy_to_stream_ex(dba->fp, fp_tmp, PHP_STREAM_COPY_ALL, NULL)) { - php_error_docref(NULL, E_WARNING, "Could not copy remainder to temporary stream"); - ret = FAILURE; - } - } + fp_tmp = php_stream_temp_create(0, 64 * 1024); + if (!fp_tmp) { + php_error_docref(NULL, E_WARNING, "Could not create temporary stream"); + goto end; + } + + php_stream_seek(dba->fp, 0, SEEK_END); + if (pos_grp_next != (size_t)php_stream_tell(dba->fp)) { + php_stream_seek(dba->fp, pos_grp_next, SEEK_SET); + if (SUCCESS != php_stream_copy_to_stream_ex(dba->fp, fp_tmp, PHP_STREAM_COPY_ALL, NULL)) { + php_error_docref(NULL, E_WARNING, "Could not copy remainder to temporary stream"); + goto end; } } /* 5 */ - if (ret == SUCCESS) { - if (!value || (key->name && strlen(key->name))) { - ret = inifile_truncate(dba, append ? pos_grp_next : pos_grp_start); /* writes error on fail */ + if (!value || (key->name && strlen(key->name))) { + bool has_succeeded = inifile_truncate(dba, is_append ? pos_grp_next : pos_grp_start); + if (!has_succeeded) { + /* Warning is already emited by inifile_truncate() on failure */ + goto end; } } - if (ret == SUCCESS) { - if (key->name && strlen(key->name)) { - /* 6 */ - if (!append && ini_tmp) { - ret = inifile_filter(dba, ini_tmp, key, found); - } - - /* 7 */ - /* important: do not query ret==SUCCESS again: inifile_filter might fail but - * however next operation must be done. - */ - if (value) { - if (pos_grp_start == pos_grp_next && key->group && strlen(key->group)) { - php_stream_printf(dba->fp, "[%s]\n", key->group); - } - php_stream_printf(dba->fp, "%s=%s\n", key->name, value->value ? value->value : ""); - } + int ret = SUCCESS; + bool is_ok = true; + if (key->name && strlen(key->name)) { + /* 6 */ + if (!is_append && ini_tmp) { + ret = inifile_filter(dba, ini_tmp, key, found); } - /* 8 */ + /* 7 */ /* important: do not query ret==SUCCESS again: inifile_filter might fail but * however next operation must be done. */ - if (fp_tmp && php_stream_tell(fp_tmp)) { - php_stream_seek(fp_tmp, 0, SEEK_SET); - php_stream_seek(dba->fp, 0, SEEK_END); - if (SUCCESS != php_stream_copy_to_stream_ex(fp_tmp, dba->fp, PHP_STREAM_COPY_ALL, NULL)) { - zend_throw_error(NULL, "Could not copy from temporary stream - ini file truncated"); - ret = FAILURE; + if (value) { + if (pos_grp_start == pos_grp_next && key->group && strlen(key->group)) { + php_stream_printf(dba->fp, "[%s]\n", key->group); } + php_stream_printf(dba->fp, "%s=%s\n", key->name, value->value ? value->value : ""); + } + } + + /* 8 */ + /* important: do not query ret==SUCCESS again: inifile_filter might fail but + * however next operation must be done. + */ + if (fp_tmp && php_stream_tell(fp_tmp)) { + php_stream_seek(fp_tmp, 0, SEEK_SET); + php_stream_seek(dba->fp, 0, SEEK_END); + if (SUCCESS != php_stream_copy_to_stream_ex(fp_tmp, dba->fp, PHP_STREAM_COPY_ALL, NULL)) { + zend_throw_error(NULL, "Could not copy from temporary stream - ini file truncated"); + ret = FAILURE; } } + end: if (ini_tmp) { php_stream_close(ini_tmp->fp); - inifile_free(ini_tmp, 0); + inifile_free(ini_tmp, /* persistent */ false); } if (fp_tmp) { php_stream_close(fp_tmp); @@ -554,37 +538,27 @@ static int inifile_delete_replace_append(inifile *dba, const key_type *key, cons } /* }}} */ -/* {{{ inifile_delete */ int inifile_delete(inifile *dba, const key_type *key) { - return inifile_delete_replace_append(dba, key, NULL, 0, NULL); + return inifile_delete_replace_append(dba, key, NULL, /* is_append */ false, NULL); } -/* }}} */ -/* {{{ inifile_delete_ex */ int inifile_delete_ex(inifile *dba, const key_type *key, bool *found) { - return inifile_delete_replace_append(dba, key, NULL, 0, found); + return inifile_delete_replace_append(dba, key, NULL, /* is_append */ false, found); } -/* }}} */ -/* {{{ inifile_relace */ int inifile_replace(inifile *dba, const key_type *key, const val_type *value) { - return inifile_delete_replace_append(dba, key, value, 0, NULL); + return inifile_delete_replace_append(dba, key, value, /* is_append */ false, NULL); } -/* }}} */ -/* {{{ inifile_replace_ex */ int inifile_replace_ex(inifile *dba, const key_type *key, const val_type *value, bool *found) { - return inifile_delete_replace_append(dba, key, value, 0, found); + return inifile_delete_replace_append(dba, key, value, /* is_append */ false, found); } -/* }}} */ -/* {{{ inifile_append */ int inifile_append(inifile *dba, const key_type *key, const val_type *value) { - return inifile_delete_replace_append(dba, key, value, 1, NULL); + return inifile_delete_replace_append(dba, key, value, /* is_append */ true, NULL); } -/* }}} */ diff --git a/ext/dba/libinifile/inifile.h b/ext/dba/libinifile/inifile.h index e931c70bcbf6b..f2fcd571d3c76 100644 --- a/ext/dba/libinifile/inifile.h +++ b/ext/dba/libinifile/inifile.h @@ -17,6 +17,9 @@ #ifndef PHP_LIB_INIFILE_H #define PHP_LIB_INIFILE_H +#include +#include + typedef struct { char *group; char *name; @@ -36,14 +39,14 @@ typedef struct { char *lockfn; int lockfd; php_stream *fp; - int readonly; + bool readonly; line_type curr; line_type next; } inifile; val_type inifile_fetch(inifile *dba, const key_type *key, int skip); -int inifile_firstkey(inifile *dba); -int inifile_nextkey(inifile *dba); +bool inifile_firstkey(inifile *dba); +bool inifile_nextkey(inifile *dba); int inifile_delete(inifile *dba, const key_type *key); int inifile_delete_ex(inifile *dba, const key_type *key, bool *found); int inifile_replace(inifile *dba, const key_type *key, const val_type *val); @@ -58,7 +61,7 @@ void inifile_key_free(key_type *key); void inifile_val_free(val_type *val); void inifile_line_free(line_type *ln); -inifile * inifile_alloc(php_stream *fp, int readonly, int persistent); -void inifile_free(inifile *dba, int persistent); +inifile * inifile_alloc(php_stream *fp, bool readonly, bool persistent); +void inifile_free(inifile *dba, bool persistent); #endif From ecc2f3a72ac53500dbc4677d8be02a907f1d4371 Mon Sep 17 00:00:00 2001 From: Gina Peter Bnayard Date: Sat, 10 Aug 2024 00:18:10 +0200 Subject: [PATCH 3/8] Fix warnings --- ext/dba/libinifile/inifile.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ext/dba/libinifile/inifile.c b/ext/dba/libinifile/inifile.c index f27b6738becc8..e27d1b51fac57 100644 --- a/ext/dba/libinifile/inifile.c +++ b/ext/dba/libinifile/inifile.c @@ -444,6 +444,7 @@ static int inifile_delete_replace_append(inifile *dba, const key_type *key, cons size_t pos_grp_start = 0, pos_grp_next; inifile *ini_tmp = NULL; php_stream *fp_tmp = NULL; + int ret = FAILURE; /* 1) Search group start * 2) Search next group @@ -490,8 +491,7 @@ static int inifile_delete_replace_append(inifile *dba, const key_type *key, cons } } - int ret = SUCCESS; - bool is_ok = true; + ret = SUCCESS; if (key->name && strlen(key->name)) { /* 6 */ if (!is_append && ini_tmp) { From ea4bf3871e9c95b31983f1f255a62360cea1afb9 Mon Sep 17 00:00:00 2001 From: Gina Peter Bnayard Date: Sat, 10 Aug 2024 00:25:25 +0200 Subject: [PATCH 4/8] one more conversion --- ext/dba/libinifile/inifile.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/ext/dba/libinifile/inifile.c b/ext/dba/libinifile/inifile.c index e27d1b51fac57..ee0d1bc9a76af 100644 --- a/ext/dba/libinifile/inifile.c +++ b/ext/dba/libinifile/inifile.c @@ -309,9 +309,9 @@ static bool inifile_truncate(inifile *dba, size_t size) /* {{{ inifile_find_group * if found pos_grp_start points to "[group_name]" */ -static int inifile_find_group(inifile *dba, const key_type *key, size_t *pos_grp_start) +static bool inifile_find_group(inifile *dba, const key_type *key, size_t *pos_grp_start) { - int ret = FAILURE; + bool found_group = false; php_stream_flush(dba->fp); php_stream_seek(dba->fp, 0, SEEK_SET); @@ -323,7 +323,7 @@ static int inifile_find_group(inifile *dba, const key_type *key, size_t *pos_grp while (inifile_read(dba, &ln)) { if (inifile_key_cmp(&ln.key, key) != DIFFERENT) { - ret = SUCCESS; + found_group = true; break; } *pos_grp_start = php_stream_tell(dba->fp); @@ -331,12 +331,12 @@ static int inifile_find_group(inifile *dba, const key_type *key, size_t *pos_grp inifile_line_free(&ln); } else { *pos_grp_start = 0; - ret = SUCCESS; + found_group = true; } - if (ret == FAILURE) { + if (!found_group) { *pos_grp_start = php_stream_tell(dba->fp); } - return ret; + return found_group; } /* }}} */ From 4a2bf5682788d63365a52e6b8b070fccaad76c2f Mon Sep 17 00:00:00 2001 From: Gina Peter Bnayard Date: Sat, 10 Aug 2024 00:35:34 +0200 Subject: [PATCH 5/8] Minor header cleanup --- ext/dba/libinifile/inifile.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/ext/dba/libinifile/inifile.c b/ext/dba/libinifile/inifile.c index ee0d1bc9a76af..63092fae680f2 100644 --- a/ext/dba/libinifile/inifile.c +++ b/ext/dba/libinifile/inifile.c @@ -14,14 +14,11 @@ +----------------------------------------------------------------------+ */ -/* $Id$ */ - #ifdef HAVE_CONFIG_H #include "config.h" #endif -#include "php.h" -#include "php_globals.h" +#include "inifile.h" #include #include @@ -30,7 +27,10 @@ #include #endif -#include "inifile.h" +#include "zend_alloc.h" +#include "php_streams.h" +#include "spprintf.h" +#include "php.h" /* ret = -1 means that database was opened for read-only * ret = 0 success From 72f2224e8e4429e5383ee5eb0763f17a109f9ff6 Mon Sep 17 00:00:00 2001 From: Gina Peter Bnayard Date: Sun, 11 Aug 2024 02:55:21 +0200 Subject: [PATCH 6/8] Fixup and refactoring --- ext/dba/libinifile/inifile.c | 76 +++++++++++++++--------------------- 1 file changed, 32 insertions(+), 44 deletions(-) diff --git a/ext/dba/libinifile/inifile.c b/ext/dba/libinifile/inifile.c index 63092fae680f2..36616ccd64afb 100644 --- a/ext/dba/libinifile/inifile.c +++ b/ext/dba/libinifile/inifile.c @@ -18,6 +18,7 @@ #include "config.h" #endif +#include "php.h" /* Headers are un utter mess so this needs to be before */ #include "inifile.h" #include @@ -30,7 +31,6 @@ #include "zend_alloc.h" #include "php_streams.h" #include "spprintf.h" -#include "php.h" /* ret = -1 means that database was opened for read-only * ret = 0 success @@ -150,8 +150,7 @@ static char *etrim(const char *str) } /* }}} */ -/* {{{ inifile_findkey */ -static int inifile_read(inifile *dba, line_type *ln) { +static bool inifile_read(inifile *dba, line_type *ln) { char *fline; char *pos; @@ -170,7 +169,7 @@ static int inifile_read(inifile *dba, line_type *ln) { ln->key.name = estrdup(""); ln->pos = php_stream_tell(dba->fp); efree(fline); - return 1; + return true; } else { efree(fline); continue; @@ -190,7 +189,7 @@ static int inifile_read(inifile *dba, line_type *ln) { ln->val.value = etrim(pos+1); ln->pos = php_stream_tell(dba->fp); efree(fline); - return 1; + return true; } else { /* simply ignore lines without '=' * those should be comments @@ -202,9 +201,8 @@ static int inifile_read(inifile *dba, line_type *ln) { } } inifile_line_free(ln); - return 0; + return false; } -/* }}} */ enum inifile_key_cmp_status { EQUAL, @@ -306,60 +304,50 @@ static bool inifile_truncate(inifile *dba, size_t size) return true; } -/* {{{ inifile_find_group - * if found pos_grp_start points to "[group_name]" - */ -static bool inifile_find_group(inifile *dba, const key_type *key, size_t *pos_grp_start) +/* Return position where which points to "[group_name]" */ +static size_t inifile_find_group(inifile *dba, const key_type *key) { - bool found_group = false; - + /* Rewind to the beginning of the file and free currently hold lines */ php_stream_flush(dba->fp); php_stream_seek(dba->fp, 0, SEEK_SET); inifile_line_free(&dba->curr); inifile_line_free(&dba->next); - if (key->group && strlen(key->group)) { - line_type ln = {{NULL,NULL},{NULL},0}; + /* If key is not in a group return the position of the start of the file */ + if (!key->group || strlen(key->group) == 0) { + return 0; + } - while (inifile_read(dba, &ln)) { - if (inifile_key_cmp(&ln.key, key) != DIFFERENT) { - found_group = true; - break; - } - *pos_grp_start = php_stream_tell(dba->fp); + size_t group_start_position = 0; + line_type ln = {{NULL,NULL},{NULL},0}; + while (inifile_read(dba, &ln)) { + if (inifile_key_cmp(&ln.key, key) != DIFFERENT) { + break; } - inifile_line_free(&ln); - } else { - *pos_grp_start = 0; - found_group = true; - } - if (!found_group) { - *pos_grp_start = php_stream_tell(dba->fp); + group_start_position = php_stream_tell(dba->fp); } - return found_group; + inifile_line_free(&ln); + return group_start_position; } -/* }}} */ -/* {{{ inifile_next_group - * only valid after a call to inifile_find_group +/* Must only be called after inifile_find_group() + * * if any next group is found pos_grp_start points to "[group_name]" or whitespace before that */ -static bool inifile_next_group(inifile *dba, const key_type *key, size_t *pos_grp_start) +static size_t inifile_next_group(inifile *dba, const key_type *key) { - bool has_next_group = false; line_type ln = {{NULL,NULL},{NULL},0}; - *pos_grp_start = php_stream_tell(dba->fp); + size_t start_position = php_stream_tell(dba->fp); ln.key.group = estrdup(key->group); while (inifile_read(dba, &ln)) { if (inifile_key_cmp(&ln.key, key) == DIFFERENT) { - has_next_group = true; break; } - *pos_grp_start = php_stream_tell(dba->fp); + start_position = php_stream_tell(dba->fp); } inifile_line_free(&ln); - return has_next_group; + return start_position; } /* }}} */ @@ -369,7 +357,7 @@ static bool inifile_copy_to(inifile *dba, size_t pos_start, size_t pos_end, inif if (pos_start == pos_end) { *ini_copy = NULL; - return false; + return true; } if ((fp = php_stream_temp_create(0, 64 * 1024)) == NULL) { php_error_docref(NULL, E_WARNING, "Could not create temporary stream"); @@ -438,10 +426,9 @@ static int inifile_filter(inifile *dba, inifile *from, const key_type *key, bool } /* }}} */ -/* {{{ inifile_delete_replace_append */ +/* If value == NULL we are deleting the entry, if is_append is true then we are inserting a new entry */ static int inifile_delete_replace_append(inifile *dba, const key_type *key, const val_type *value, bool is_append, bool *found) { - size_t pos_grp_start = 0, pos_grp_next; inifile *ini_tmp = NULL; php_stream *fp_tmp = NULL; int ret = FAILURE; @@ -457,11 +444,12 @@ static int inifile_delete_replace_append(inifile *dba, const key_type *key, cons * 8) Append temporary stream */ - assert(!is_append || (key->name && value)); /* missuse */ + /* Check missuse of API */ + assert(!is_append || (key->name && value)); /* 1 - 3 */ - inifile_find_group(dba, key, &pos_grp_start); - inifile_next_group(dba, key, &pos_grp_next); + size_t pos_grp_start = inifile_find_group(dba, key); + size_t pos_grp_next = inifile_next_group(dba, key); if (!is_append && !inifile_copy_to(dba, pos_grp_start, pos_grp_next, &ini_tmp)) { goto end; } From 129560462e2dbc1fc7b3c152508c51a0164845f6 Mon Sep 17 00:00:00 2001 From: Gina Peter Bnayard Date: Sun, 11 Aug 2024 03:38:31 +0200 Subject: [PATCH 7/8] Nits --- ext/dba/libinifile/inifile.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/ext/dba/libinifile/inifile.c b/ext/dba/libinifile/inifile.c index 36616ccd64afb..64536d8e2aef3 100644 --- a/ext/dba/libinifile/inifile.c +++ b/ext/dba/libinifile/inifile.c @@ -472,9 +472,8 @@ static int inifile_delete_replace_append(inifile *dba, const key_type *key, cons /* 5 */ if (!value || (key->name && strlen(key->name))) { - bool has_succeeded = inifile_truncate(dba, is_append ? pos_grp_next : pos_grp_start); - if (!has_succeeded) { - /* Warning is already emited by inifile_truncate() on failure */ + if (!inifile_truncate(dba, is_append ? pos_grp_next : pos_grp_start)) { + /* Warning is already emitted by inifile_truncate() on failure */ goto end; } } @@ -511,7 +510,7 @@ static int inifile_delete_replace_append(inifile *dba, const key_type *key, cons } } - end: +end: if (ini_tmp) { php_stream_close(ini_tmp->fp); inifile_free(ini_tmp, /* persistent */ false); From dae8afbe44f7954f6a5f0a6c8e054c3fe789d2f7 Mon Sep 17 00:00:00 2001 From: Gina Peter Bnayard Date: Sun, 11 Aug 2024 15:48:02 +0200 Subject: [PATCH 8/8] Fix Windows Thanks to @cmb69 --- ext/dba/tests/dba_inifile_group_keys.phpt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/dba/tests/dba_inifile_group_keys.phpt b/ext/dba/tests/dba_inifile_group_keys.phpt index eb305275749ea..b435c44f68093 100644 --- a/ext/dba/tests/dba_inifile_group_keys.phpt +++ b/ext/dba/tests/dba_inifile_group_keys.phpt @@ -12,7 +12,7 @@ check_skip('inifile'); require_once __DIR__ . '/setup/setup_dba_tests.inc'; $name = __DIR__ . '/inifile_group_keys_tests.db'; -$db = dba_open($name, 'c', 'inifile'); +$db = dba_open($name, 'c-', 'inifile'); echo "Checks on an empty database:\n"; var_dump(dba_delete(['group', 'name'], $db));