On (17/03/14 22:00), Lukas Slebodnik wrote: >On (08/03/14 19:11), Dmitri Pal wrote: >>Hello, >> >>The following patches address the request to be able to read config >>data from a memory buffer rather than a file. >>Ticket: https://fedorahosted.org/sssd/ticket/2267 >> >>It also addresses a build problem that I found on my system and fixes >>a tight loop in parsing when we ignore errors and file is empty. >> >> >>-- >>Thank you, >>Dmitri Pal >> >>Sr. Engineering Manager for IdM portfolio >>Red Hat Inc. >> >> >>------------------------------- >>Looking to carve out IT costs? >>www.redhat.com/carveoutcosts/ >> >> >> > >>From 30a92ababaa9148a7354ebf52bf318d37b5451d2 Mon Sep 17 00:00:00 2001 >>From: Dmitri Pal <d...@redhat.com> >>Date: Sat, 8 Mar 2014 19:00:57 -0500 >>Subject: [PATCH 2/4] [INI] New entry to read data from mem >> >>The commit adds a new entry point to read configuration >>data from a memory buffer instead of file. >>--- >> ini/ini_configobj.h | 25 ++++++ >> ini/ini_fileobj.c | 204 >> +++++++++++++++++++++++++++++++++++++------------ >> ini/libini_config.sym | 1 + >> 3 files changed, 182 insertions(+), 48 deletions(-) >> >>diff --git a/ini/ini_configobj.h b/ini/ini_configobj.h >>index 9a4eb58..4ef98dc 100644 >>--- a/ini/ini_configobj.h >>+++ b/ini/ini_configobj.h >>@@ -483,6 +483,31 @@ int ini_config_file_open(const char *filename, >> struct ini_cfgfile **file_ctx); >> >> /** >>+ * @brief Create a configuration file object using memory buffer. >>+ * >>+ * Create a file object for parsing a configuration file. >>+ * Configuration will use provided memory instead of the actual file. >>+ * >>+ * A "configuration file object" is different from >>+ * a "configuration object". The former stores metadata >>+ * about a file the configuration data is read from, >>+ * while the latter holds the configuration itself. >>+ * >>+ * @param[in] data_buf In memory configuration data. >>+ * Needs to be NULL terminated. > ^^^^^^^^^^^^^^^^ > In my opinion, it is little bit confusing. > Because argument data_buf has type "void *". > What about: byte array null byte ('\0') byte array terminated with null byte ('\0') > >Do we really need null byte terminated string? >I think fmemopen will need null byte terminated string only if buffer is opened >in append mode. > >>+ * @param[in] data_len Length of memory data >>+ * not including terminating NULL. >>+ * @param[out] file_ctx Configuration file object. >>+ * >>+ * @return 0 - Success. >>+ * @return EINVAL - Invalid parameter. >>+ * @return ENOMEM - No memory. >>+ */ >>+int ini_config_file_from_mem(void *data_buf, >>+ uint32_t data_len, >>+ struct ini_cfgfile **file_ctx); >>+ > >>+/** >> * @brief Close configuration file after parsing >> * >> * Closes file but keeps the context. File can be reopened >>diff --git a/ini/ini_fileobj.c b/ini/ini_fileobj.c >>index d87956a..fbb1a81 100644 >>--- a/ini/ini_fileobj.c >>+++ b/ini/ini_fileobj.c >>@@ -46,6 +46,12 @@ enum index_utf_t { >> INDEX_UTF8 = 4 >> }; >> >>+/* Trick to work around the fact that >>+ * fmemopen failes if buffer is 0 length >>+ */ >>+unsigned char alt_buffer[2] = {0, 0}; > {'\n', 0}; >It would be better to have string with length 1. >(gdb) print strlen("\n") >$1 = 1 > >>+uint32_t alt_buffer_len = 1; >Could you use static modifier (both variable)? > >sh-4.2$ nm ini/ini_fileobj.o | grep " D " >0000000000000004 D alt_buffer >0000000000000000 D alt_buffer_len > >It is not big problem because symbols will be hidden by version-script file. >But It it will be better to hide them explicitly. > >> /* Close file but not destroy the object */ >> void ini_config_file_close(struct ini_cfgfile *file_ctx) >> { >>@@ -138,7 +144,7 @@ static enum index_utf_t check_bom(enum index_utf_t ind, >> return ind; >> } >> >>-static int read_chunk(int raw_file, size_t left, size_t increment, >>+static int read_chunk(FILE *file, size_t left, size_t increment, >> char *position, size_t *read_num) >> { >> int error = EOK; >>@@ -149,22 +155,28 @@ static int read_chunk(int raw_file, size_t left, size_t >>increment, >> >> to_read = how_much_to_read(left, increment); >> >>- TRACE_INFO_NUMBER("About to read", to_read); >>- errno = 0; >>- read_cnt = read(raw_file, position, to_read); >>- if (read_cnt == -1) { >>- error = errno; >>- TRACE_ERROR_NUMBER("Failed to read data from file", error); >>- return error; >>- } >>+ TRACE_INFO_NUMBER("About to read", (unsigned int)to_read); > ^^^^^^^^^^^^^^ > casting is not necessary in this case. > because in macro TRACE_NUMBER variable is casted to > ((unsigned long int)) >> >>- if (read_cnt != to_read) { >>- error = EIO; >>- TRACE_ERROR_NUMBER("Read less than required", error); >>- return error; >>+ Read_cnt = fread(position, to_read, 1, file); >>+ >>+ TRACE_INFO_NUMBER("Read", (unsigned int)(read_cnt * to_read)); > ^^^^^^^^^^^^^^ > the same here. >>+ >>+ if (read_cnt == 0) { >>+ error = ferror(file); >>+ if (error) { >>+ TRACE_ERROR_NUMBER("Failed to read data from file", error); >>+ return error; >>+ } >>+ error = feof(file); >>+ if(error) { >>+ TRACE_FLOW_EXIT(); >>+ return EOK; >>+ } >It would be better to call feof before ferror. > >>+ TRACE_ERROR_NUMBER("Failed to read data from file", EIO); >>+ return EIO; >> } >> >>- *read_num = read_cnt; >>+ *read_num = to_read; >> >> TRACE_FLOW_EXIT(); >> return error; >>@@ -229,7 +241,9 @@ static int initialize_conv(unsigned char *read_buf, >> } >> >> /* Internal conversion part */ >>-static int common_file_convert(int raw_file, struct ini_cfgfile *file_ctx) >>+static int common_file_convert(FILE *file, >>+ struct ini_cfgfile *file_ctx, >>+ uint32_t size) >> { >> int error = EOK; >> size_t read_cnt = 0; >>@@ -249,8 +263,8 @@ static int common_file_convert(int raw_file, struct >>ini_cfgfile *file_ctx) >> >> do { >> /* print_buffer(read_buf, ICONV_BUFFER); */ >>- error = read_chunk(raw_file, >>- file_ctx->file_stats.st_size - total_read, >>+ error = read_chunk(file, >>+ size - total_read, >> ICONV_BUFFER - in_buffer, >> read_buf + in_buffer, >> &read_cnt); >>@@ -305,9 +319,8 @@ static int common_file_convert(int raw_file, struct >>ini_cfgfile *file_ctx) >> /* We need to just read more if we can */ >> TRACE_INFO_NUMBER("Incomplete sequence len", >> src - read_buf); >>- TRACE_INFO_NUMBER("File size.", >>- file_ctx->file_stats.st_size); >>- if (total_read == file_ctx->file_stats.st_size) { >>+ TRACE_INFO_NUMBER("File size.", size); >>+ if (total_read == size) { >> /* Or return error if we can't */ >> TRACE_ERROR_NUMBER("Incomplete sequence", error); >> iconv_close(conv); >>@@ -356,7 +369,7 @@ static int common_file_convert(int raw_file, struct >>ini_cfgfile *file_ctx) >> } >> while (1); >> } >>- while (total_read < file_ctx->file_stats.st_size); >>+ while (total_read < size); >> >> iconv_close(conv); >> >>@@ -365,8 +378,7 @@ static int common_file_convert(int raw_file, struct >>ini_cfgfile *file_ctx) >> (char *)simplebuffer_get_vbuf(file_ctx->file_data)); >> TRACE_INFO_NUMBER("File len", >> simplebuffer_get_len(file_ctx->file_data)); >>- TRACE_INFO_NUMBER("Size", >>- file_ctx->file_data->size); >>+ TRACE_INFO_NUMBER("Size", size); >> errno = 0; >> file_ctx->file = fmemopen(simplebuffer_get_vbuf(file_ctx->file_data), >> simplebuffer_get_len(file_ctx->file_data), >>@@ -383,59 +395,98 @@ static int common_file_convert(int raw_file, struct >>ini_cfgfile *file_ctx) >> >> >> /* Internal common initialization part */ >>-static int common_file_init(struct ini_cfgfile *file_ctx) >>+static int common_file_init(struct ini_cfgfile *file_ctx, >>+ void *data_buf, >>+ uint32_t data_len) >> { >> int error = EOK; >>- int raw_file = 0; >>+ FILE *file = NULL; >> int stat_ret = 0; >>+ uint32_t size = 0; >>+ void *internal_data = NULL; >>+ uint32_t internal_len = 0; >> >> TRACE_FLOW_ENTRY(); >> >>- TRACE_INFO_STRING("File", file_ctx->filename); >>+ if (data_buf) { >> >>- /* Open file in binary mode first */ >>- errno = 0; >>- raw_file = open(file_ctx->filename, O_RDONLY); >>- if (raw_file == -1) { >>- error = errno; >>- TRACE_ERROR_NUMBER("Failed to open file in binary mode", error); >>- return error; >>+ if(data_len) { >>+ internal_data = data_buf; >>+ internal_len = data_len; >>+ } >>+ else { >>+ /* If buffer is empty fmemopen will return an error. >>+ * This will prevent creation of adefault config object. >>+ * Instead we will use buffer that has at least one character. */ >>+ internal_data = (void *)alt_buffer; > ^^^^^^^^ > Any pointer can be stored to variable with type "void *" > without explicit casting. > >>+ internal_len = alt_buffer_len; >>+ } >>+ >>+ TRACE_INFO_NUMBER("Inside file_init len", internal_len); >>+ TRACE_INFO_STRING("Inside file_init data:", (char *)internal_data); >>+ >>+ file = fmemopen(internal_data, internal_len, "r"); >>+ if (!file) { >>+ error = errno; >>+ TRACE_ERROR_NUMBER("Failed to memmap file", error); >>+ return error; >>+ } >>+ size = internal_len; >> } >>+ else { >> >>- /* Get the size of the file */ >>- errno = 0; >>- stat_ret = fstat(raw_file, &(file_ctx->file_stats)); >>- if (stat_ret == -1) { >>- error = errno; >>- close(raw_file); >>- TRACE_ERROR_NUMBER("Failed to get file stats", error); >>- return error; >>+ TRACE_INFO_STRING("File", file_ctx->filename); >>+ >>+ /* Open file to get its size */ >>+ errno = 0; >>+ file = fopen(file_ctx->filename, "r"); >>+ if (!file) { >>+ error = errno; >>+ TRACE_ERROR_NUMBER("Failed to open file", error); >>+ return error; >>+ } >>+ >>+ /* Get the size of the file */ >>+ errno = 0; >>+ stat_ret = fstat(fileno(file), &(file_ctx->file_stats)); >>+ if (stat_ret == -1) { >>+ error = errno; >>+ fclose(file); >>+ TRACE_ERROR_NUMBER("Failed to get file stats", error); >>+ return error; >>+ } >>+ size = file_ctx->file_stats.st_size; >> } >> >> /* Trick to overcome the fact that >> * fopen and fmemopen behave differently when file >> * is 0 length >> */ >>- if (file_ctx->file_stats.st_size) { >>- error = common_file_convert(raw_file, file_ctx); >>- close(raw_file); >>+ if (size) { >>+ error = common_file_convert(file, file_ctx, size); >> if (error) { >> TRACE_ERROR_NUMBER("Failed to convert file", >> error); >>+ fclose(file); >> return error; >> } >> } >> else { >>+ >>+ TRACE_INFO_STRING("File is 0 length",""); >> errno = 0; >>- file_ctx->file = fdopen(raw_file, "r"); >>+ >>+ file_ctx->file = fdopen(fileno(file), "r"); >> if (!(file_ctx->file)) { >> error = errno; >>- close(raw_file); >>+ fclose(file); >> TRACE_ERROR_NUMBER("Failed to fdopen file", error); >> return error; >> } >> } >> >>+ fclose(file); >>+ >> /* Collect stats */ >> if (file_ctx->metadata_flags & INI_META_STATS) { >> file_ctx->stats_read = 1; >>@@ -505,7 +556,7 @@ int ini_config_file_open(const char *filename, >> } >> >> /* Do common init */ >>- error = common_file_init(new_ctx); >>+ error = common_file_init(new_ctx, NULL, 0); >> if(error) { >> TRACE_ERROR_NUMBER("Failed to do common init", error); >> ini_config_file_destroy(new_ctx); >>@@ -517,6 +568,63 @@ int ini_config_file_open(const char *filename, >> return error; >> } >> >>+/* Create a file object from a memory buffer */ >>+int ini_config_file_from_mem(void *data_buf, >>+ uint32_t data_len, >>+ struct ini_cfgfile **file_ctx) >>+{ >>+ int error = EOK; >>+ struct ini_cfgfile *new_ctx = NULL; >>+ >>+ TRACE_FLOW_ENTRY(); >>+ >>+ if ((!data_buf) || (!file_ctx)) { >>+ TRACE_ERROR_NUMBER("Invalid parameter.", EINVAL); >>+ return EINVAL; >>+ } >>+ >>+ /* Allocate structure */ >>+ new_ctx = malloc(sizeof(struct ini_cfgfile)); >>+ if (!new_ctx) { >>+ TRACE_ERROR_NUMBER("Failed to allocate file ctx.", ENOMEM); >>+ return ENOMEM; >>+ } >>+ >>+ new_ctx->filename = NULL; >>+ new_ctx->file = NULL; >>+ new_ctx->file_data = NULL; >>+ new_ctx->metadata_flags = 0; >Could we use calloc instead of malloc. >Memory will be set to zero autmatically. We could remove these lines in this >case. > >>+ >>+ error = simplebuffer_alloc(&(new_ctx->file_data)); >>+ if (error) { >>+ TRACE_ERROR_NUMBER("Failed to allocate buffer ctx.", error); >>+ ini_config_file_destroy(new_ctx); >>+ return error; >>+ } >>+ >>+ /* Put an empty string into the file name */ >>+ new_ctx->filename = strdup(""); >>+ if (!(new_ctx->filename)) { >>+ ini_config_file_destroy(new_ctx); >>+ TRACE_ERROR_NUMBER("Failed to put empty string into filename.", >>ENOMEM); >>+ return ENOMEM; >>+ } >>+ >>+ /* Do common init */ >>+ error = common_file_init(new_ctx, data_buf, data_len); >>+ if(error) { >>+ TRACE_ERROR_NUMBER("Failed to do common init", error); >>+ ini_config_file_destroy(new_ctx); >>+ return error; >>+ } >>+ >>+ *file_ctx = new_ctx; >>+ TRACE_FLOW_EXIT(); >>+ return error; >>+} >>+ >>+ >>+ >> /* Create a file object from existing one */ >> int ini_config_file_reopen(struct ini_cfgfile *file_ctx_in, >> struct ini_cfgfile **file_ctx_out) >>@@ -564,7 +672,7 @@ int ini_config_file_reopen(struct ini_cfgfile >>*file_ctx_in, >> } >> >> /* Do common init */ >>- error = common_file_init(new_ctx); >>+ error = common_file_init(new_ctx, NULL, 0); >> if(error) { >> TRACE_ERROR_NUMBER("Failed to do common init", error); >> ini_config_file_destroy(new_ctx); >>diff --git a/ini/libini_config.sym b/ini/libini_config.sym >>index d38b17b..d0035f2 100644 >>--- a/ini/libini_config.sym >>+++ b/ini/libini_config.sym >>@@ -46,6 +46,7 @@ global: >> ini_config_destroy; >> ini_config_clean_state; >> ini_config_file_open; >>+ ini_config_file_from_mem; >> ini_config_file_close; >> ini_config_file_reopen; >> ini_config_file_destroy; >>-- >>1.7.1 >> > >>From 16c9140ada8461250d2aa8ac484e29c5237fdeaa Mon Sep 17 00:00:00 2001 >>From: Dmitri Pal <d...@redhat.com> >>Date: Sat, 8 Mar 2014 19:04:02 -0500 >>Subject: [PATCH 4/4] [INI] Unit test for the new interface >> >>This patch adds unit test for the interface that allows >>reading configuration data from memory. >>--- >> ini/ini_parse_ut.c | 154 >> +++++++++++++++++++++++++++++++++++++++++++++++----- >> 1 files changed, 140 insertions(+), 14 deletions(-) >> >>diff --git a/ini/ini_parse_ut.c b/ini/ini_parse_ut.c >>index 1fa2624..721ee4a 100644 >>--- a/ini/ini_parse_ut.c >>+++ b/ini/ini_parse_ut.c >>@@ -50,7 +50,8 @@ typedef int (*test_fn)(void); >> >> static int test_one_file(const char *in_filename, >> const char *out_filename, >>- int edge) >>+ int edge, >>+ int mem) > I think type bool would be better. For example: bool from_memory > In my opinion, it is easier to read code: "if (from_memory)" > type bool is just a cosmetic change. (it needn't be changed) > > >> { >> int error = EOK; >> struct ini_cfgfile *file_ctx = NULL; >>@@ -60,6 +61,11 @@ static int test_one_file(const char *in_filename, >> char **error_list = NULL; >> struct simplebuffer *sbobj = NULL; >> uint32_t left = 0; >>+ uint32_t stream_len = 0; >>+ void *stream_data = NULL; >>+ struct stat file_stats; >>+ int stat_ret = 0; >>+ FILE *file = NULL; >> >> INIOUT(printf("<==== Testing file %s ====>\n", in_filename)); >> >>@@ -70,14 +76,73 @@ static int test_one_file(const char *in_filename, >> return error; >> } >> >>- error = ini_config_file_open(in_filename, >>- 0, /* TBD */ >>- &file_ctx); >>- if (error) { >>- printf("Failed to open file %s for reading. Error %d.\n", >>- in_filename, error); >>- ini_config_destroy(ini_config); >>- return error; >>+ if(mem) { > ^^^ >You forget to add space between "if" and the bracket. > >>+ /* Get file stats */ >>+ errno = 0; >>+ stat_ret = stat(in_filename, &file_stats); >>+ if (stat_ret == -1) { > ^^^^ > with space >>+ error = errno; >>+ printf("Failed to get file stats. Error %d.\n", error); >>+ ini_config_destroy(ini_config); >>+ return error; >>+ } >>+ /* Allocate memory to store file */ >>+ errno = 0; >>+ stream_len = file_stats.st_size; >>+ stream_data = malloc(file_stats.st_size + 1); >>+ if(!stream_data) { > ^^^^ > without space >>+ error = errno; >>+ printf("Failed to allocate memory for stream. Error %d.\n", >>error); >>+ ini_config_destroy(ini_config); >>+ return error; >>+ } >>+ *((unsigned char *)(stream_data) + stream_len) = '\0'; >>+ /* Open file */ >>+ errno = 0; >>+ file = fopen(in_filename, "r"); >>+ if(!stream_data) { > ^^^^ > without space >>+ error = errno; >>+ printf("Failed to open file to prepare stream. Error %d.\n", >>error); >>+ free(stream_data); >>+ ini_config_destroy(ini_config); >>+ return error; >>+ } >>+ /* Read file into memory */ >>+ errno = 0; >>+ fread(stream_data, stream_len, 1, file); >>+ error = ferror(file); >>+ if(error) { > ^^^^ > without space >>+ printf("Failed to read stream data. Error %d.\n", error); >>+ free(stream_data); >>+ fclose(file); >>+ ini_config_destroy(ini_config); >>+ return error; >>+ } >>+ fclose(file); >>+ >>+ INIOUT(printf("Data len %u\n", stream_len)); >>+ INIOUT(printf("Data:\n%s\n", (char *)stream_data)); >>+ >>+ error = ini_config_file_from_mem(stream_data, >>+ stream_len, >>+ &file_ctx); >>+ if (error) { >>+ printf("Failed to open from memory. Error %d.\n", error); >>+ free(stream_data); >>+ ini_config_destroy(ini_config); >>+ return error; >>+ } >>+ } >>+ else { >>+ error = ini_config_file_open(in_filename, >>+ 0, /* TBD */ >>+ &file_ctx); >>+ if (error) { >>+ printf("Failed to open file %s for reading. Error %d.\n", >>+ in_filename, error); >>+ ini_config_destroy(ini_config); >>+ return error; >>+ } >> } >> >> error = ini_config_parse(file_ctx, >>@@ -89,15 +154,29 @@ static int test_one_file(const char *in_filename, >> INIOUT(printf("Failed to parse configuration. Error %d.\n", error)); >> >> if (ini_config_error_count(ini_config)) { >>- INIOUT(printf("Errors detected while parsing: %s\n", >>- ini_config_get_filename(file_ctx))); >>+ if (mem) { >>+ INIOUT(printf("Errors detected while parsing" >>+ " configuration stored in memory: %s\n", >>+ in_filename)); >>+ } >>+ else { >>+ INIOUT(printf("Errors detected while parsing: %s\n", >>+ ini_config_get_filename(file_ctx))); >>+ } >> ini_config_get_errors(ini_config, &error_list); >> INIOUT(ini_config_print_errors(stdout, error_list)); >> ini_config_free_errors(error_list); >> } >>- /* We do not return here intentionally */ >>+ /* If we are testing memory mapped, return error */ >>+ if (mem) { >>+ free(stream_data); >>+ ini_config_file_destroy(file_ctx); >>+ ini_config_destroy(ini_config); >>+ return error; >>+ } >> } >> >>+ if (mem) free(stream_data); >> ini_config_file_destroy(file_ctx); >> >> INIOUT(col_debug_collection(ini_config->cfg, COL_TRAVERSE_DEFAULT)); >>@@ -200,8 +279,9 @@ static int read_save_test(void) >> snprintf(infile, PATH_MAX, "%s/ini/ini.d/%s.conf", >> (srcdir == NULL) ? "." : srcdir, files[i]); >> snprintf(outfile, PATH_MAX, "./%s_%d.conf.out", files[i], edge); >>- error = test_one_file(infile, outfile, edge); >>+ error = test_one_file(infile, outfile, edge, 0); >> INIOUT(printf("Test for file: %s returned %d\n", files[i], >> error)); >>+ if (error) return error; >> } >> i++; >> } >>@@ -211,6 +291,51 @@ static int read_save_test(void) >> return EOK; >> } >> >>+ >>+/* Run tests for multiple files */ >>+static int read_mem_test(void) >>+{ >>+ int error = EOK; >>+ int i = 0; >>+ int edge = 5; >>+ char infile[PATH_MAX]; >>+ char outfile[PATH_MAX]; >>+ char *srcdir = NULL; >>+ const char *files[] = { "real", >>+ "mysssd", >>+ "ipa", >>+ "test", >>+ "smerge", >>+ "real8", >>+ "real16be", >>+ "real16le", >>+ "real32be", >>+ "real32le", >>+ "symbols", >I am attaching two files: empty.conf (size is zero) > new_line.conf (size is one and contains only '\n' >Could you add them to tests? > >>+ NULL }; >>+ >>+ INIOUT(printf("<==== Read mem test ====>\n")); >>+ >>+ srcdir = getenv("srcdir"); >>+ >>+ while(files[i]) { >>+ for ( edge = 15; edge < 100; edge +=25) { >>+ snprintf(infile, PATH_MAX, "%s/ini/ini.d/%s.conf", >>+ (srcdir == NULL) ? "." : srcdir, files[i]); >>+ snprintf(outfile, PATH_MAX, "./%s_%d.conf.mem.out", files[i], >>edge); >>+ error = test_one_file(infile, outfile, edge, 1); >>+ INIOUT(printf("Test for file: %s returned %d\n", files[i], >>error)); >>+ if ((error) && (strncmp(files[i], "test", 4) != 0)) return error; >>+ } >>+ i++; >>+ } >>+ >>+ INIOUT(printf("<==== Read mem test end ====>\n")); >>+ >>+ return EOK; >>+} >>+ >>+ >> /* Run tests for multiple files */ >> static int read_again_test(void) >> { >>@@ -241,7 +366,7 @@ static int read_again_test(void) >> snprintf(infile, PATH_MAX, "./%s_%d.conf.out", files[i], edge); >> snprintf(outfile, PATH_MAX, "./%s_%d.conf.2.out", files[i], >> edge); >> >>- error = test_one_file(infile, outfile, edge); >>+ error = test_one_file(infile, outfile, edge, 0); >> INIOUT(printf("Test for file: %s returned %d\n", files[i], >> error)); >> if (error) break; >> snprintf(command, PATH_MAX * 3, "diff -q %s %s", infile, >> outfile); >>@@ -3058,6 +3183,7 @@ int main(int argc, char *argv[]) >> int error = 0; >> test_fn tests[] = { read_save_test, >> read_again_test, >>+ read_mem_test, >> merge_values_test, >> merge_section_test, >> merge_file_test, >>-- >>1.7.1 >> > >>From: Dmitri Pal <d...@redhat.com> >>Date: Sat, 8 Mar 2014 19:02:29 -0500 >>Subject: [PATCH 3/4] [INI] Prevent tight loop >> >>New functionality added revealed a potential tight loop >>with files of the 0 length. This patch addresses this issue. >>--- >> ini/ini_parse.c | 24 +++++++++++++++--------- >> 1 files changed, 15 insertions(+), 9 deletions(-) >> >>diff --git a/ini/ini_parse.c b/ini/ini_parse.c >>index 7792324..0bab822 100644 >>--- a/ini/ini_parse.c >>+++ b/ini/ini_parse.c >>@@ -1534,16 +1534,22 @@ static int parser_error(struct parser_obj *po) >> else po->ret = EIO; >> } >> else if (po->error_level == INI_STOP_ON_NONE) { >>- action = PARSE_READ; >>- if (po->ret == 0) { >>- if (po->last_error & INI_WARNING) po->ret = EILSEQ; >>- else po->ret = EIO; >>+ if (po->last_error != ERR_READ) { >>+ action = PARSE_READ; >>+ if (po->ret == 0) { >>+ if (po->last_error & INI_WARNING) po->ret = EILSEQ; >>+ else po->ret = EIO; >>+ } >>+ /* It it was warning but now if it is an error >>+ * bump to return code to indicate error. */ >>+ else if((po->ret == EILSEQ) && >>+ (!(po->last_error & INI_WARNING))) po->ret = EIO; >>+ } >>+ else { >>+ /* Avoid endless loop */ >>+ action = PARSE_DONE; >>+ po->ret = EIO; >> } >>- /* It it was warning but now if it is an error >>- * bump to return code to indicate error. */ >>- else if((po->ret == EILSEQ) && >>- (!(po->last_error & INI_WARNING))) po->ret = EIO; >>- >> } >> else { /* Stop on error */ >> if (po->last_error & INI_WARNING) { > >ACK to this patch. > >LS
> >_______________________________________________ >sssd-devel mailing list >sssd-devel@lists.fedorahosted.org >https://lists.fedorahosted.org/mailman/listinfo/sssd-devel _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
