Hi, Please find a series of patches related to $subj.
0001 - Trace improvements. See patch comments. (independent patch) 0002 - Dhash improvement. See patch comments. (independent patch) 0003 - Collection enhancements needed for ELAPI 0006 - Cleaning FIXME comments in collection (ticket #157) These patches need to be applied in order. 0007 - Cleaning FIXME comment in INI. See patch comments. (independent patch) Patches 0004 and 0005 are sent in separate mails due to the size constraints. -- Thank you, Dmitri Pal Engineering Manager IPA project, Red Hat Inc. ------------------------------- Looking to carve out IT costs? www.redhat.com/carveoutcosts/
From 0ca569ece0e3bc452c8a8648b90dab45606c9958 Mon Sep 17 00:00:00 2001 From: Dmitri Pal <[email protected]> Date: Sun, 25 Oct 2009 13:21:24 -0400 Subject: [PATCH] COLLECTION Create reference to the top level collection This patch adds ability to create a reference to the top level collection. Previously one could get reference only to collection inside other collection. With this change it becomes possible to have two pointers to the same top level collection from multiple places. COLLECTION Adding comment. COLLECTION: Some tracing --- common/collection/collection.c | 49 ++++++++++++++++++++++-------------- common/collection/collection.h | 6 +++- common/collection/collection_ut.c | 13 ++++++++++ 3 files changed, 47 insertions(+), 21 deletions(-) diff --git a/common/collection/collection.c b/common/collection/collection.c index 27dee83..c8a0077 100644 --- a/common/collection/collection.c +++ b/common/collection/collection.c @@ -1171,7 +1171,9 @@ static void col_delete_collection(struct collection_item *ci) return; } - TRACE_INFO_STRING("Real work to do",""); + TRACE_INFO_STRING("Real work to do", ""); + TRACE_INFO_STRING("Property", ci->property); + TRACE_INFO_NUMBER("Next item", ci->next); col_delete_collection(ci->next); @@ -2247,8 +2249,11 @@ void col_destroy_collection(struct collection_item *ci) return; } + TRACE_INFO_STRING("Name:", ci->property); + /* Collection can be referenced by other collection */ header = (struct collection_header *)(ci->data); + TRACE_INFO_NUMBER("Reference count:", header->reference_count); if (header->reference_count > 1) { TRACE_INFO_STRING("Dereferencing a referenced collection.", ""); header->reference_count--; @@ -2376,29 +2381,35 @@ int col_get_collection_reference(struct collection_item *ci, if ((ci == NULL) || (ci->type != COL_TYPE_COLLECTION) || - (acceptor == NULL) || - (collection_to_find == NULL)) { + (acceptor == NULL)) { TRACE_ERROR_NUMBER("Invalid parameter - returning error",EINVAL); return EINVAL; } - /* Find a sub collection */ - TRACE_INFO_STRING("We are given subcollection name - search it:", - collection_to_find); - error = col_find_item_and_do(ci, collection_to_find, - COL_TYPE_COLLECTIONREF, - COL_TRAVERSE_DEFAULT, - col_get_subcollection, - (void *)(&subcollection), - COLLECTION_ACTION_FIND); - if (error) { - TRACE_ERROR_NUMBER("Search failed returning error", error); - return error; - } + if (collection_to_find) { + /* Find a sub collection */ + TRACE_INFO_STRING("We are given subcollection name - search it:", + collection_to_find); + error = col_find_item_and_do(ci, collection_to_find, + COL_TYPE_COLLECTIONREF, + COL_TRAVERSE_DEFAULT, + col_get_subcollection, + (void *)(&subcollection), + COLLECTION_ACTION_FIND); + if (error) { + TRACE_ERROR_NUMBER("Search failed returning error", error); + return error; + } - if (subcollection == NULL) { - TRACE_ERROR_STRING("Search for subcollection returned NULL pointer", ""); - return ENOENT; + if (subcollection == NULL) { + TRACE_ERROR_STRING("Search for subcollection returned NULL pointer", ""); + return ENOENT; + } + } + else { + /* Create reference to the same collection */ + TRACE_INFO_STRING("Creating reference to the top level collection.", ""); + subcollection = ci; } header = (struct collection_header *)subcollection->data; diff --git a/common/collection/collection.h b/common/collection/collection.h index 2e2fe64..665d5f0 100644 --- a/common/collection/collection.h +++ b/common/collection/collection.h @@ -686,8 +686,10 @@ int col_is_item_in_collection(struct collection_item *ci, /* Collection to fin int *found); /* Boolean that turns to nonzero if the match is found */ -/* Get collection - get a pointer to a collection included into another collection */ -/* Delete extracted collection after use to decrease reference count. */ +/* Get collection - get a pointer to a collection included into another collection. + * If the collection_to_find is NULL function reterns a reference to the top level collection. + * Delete extracted collection after use to decrease reference count. + */ int col_get_collection_reference(struct collection_item *ci, /* High level collection */ struct collection_item **acceptor, /* The pointer that will accept extracted handle */ const char *collection_to_find); /* Name to of the collection */ diff --git a/common/collection/collection_ut.c b/common/collection/collection_ut.c index 1e91c23..8662055 100644 --- a/common/collection/collection_ut.c +++ b/common/collection/collection_ut.c @@ -33,6 +33,7 @@ int ref_collection_test(void) { struct collection_item *peer = NULL; struct collection_item *socket = NULL; + struct collection_item *socket2 = NULL; char binary_dump[] = { 0, 1, 2, 3, 4, 5, 6, 7, 8 }; int error = EOK; @@ -98,7 +99,19 @@ int ref_collection_test(void) col_destroy_collection(peer); col_debug_collection(socket, COL_TRAVERSE_DEFAULT); + + error = col_get_collection_reference(socket, &socket2, NULL); + if (error) { + col_destroy_collection(socket); + printf("Failed to extract collection. Error %d\n", error); + return error; + } + + col_debug_collection(socket2, COL_TRAVERSE_DEFAULT); col_destroy_collection(socket); + col_debug_collection(socket2, COL_TRAVERSE_DEFAULT); + col_destroy_collection(socket2); + TRACE_FLOW_NUMBER("ref_collection_test. Returning", error); printf("\n\nEND OF REF TEST!!!.\n\n\n"); -- 1.5.5.6
From 593e4c9a0676be0bba2dfec834f8325b23540400 Mon Sep 17 00:00:00 2001 From: Dmitri Pal <[email protected]> Date: Mon, 7 Dec 2009 09:24:32 -0500 Subject: [PATCH] INI: Cleaning FIXME comments. Added configurable key length. Changed comments for the functions that are currently not used and reserved for future functionality. --- common/ini/configure.ac | 2 ++ common/ini/ini_config.c | 16 ++++++++++++---- common/ini/ini_config.h | 5 ++--- 3 files changed, 16 insertions(+), 7 deletions(-) diff --git a/common/ini/configure.ac b/common/ini/configure.ac index c599124..ef8f0de 100644 --- a/common/ini/configure.ac +++ b/common/ini/configure.ac @@ -21,5 +21,7 @@ AC_ARG_ENABLE([trace], [trace_level="0"]) AS_IF([test ["$trace_level" -gt "0"] -a ["$trace_level" -lt "8"] ],[AC_SUBST([TRACE_VAR],["-DTRACE_LEVEL=$trace_level"])]) +AC_DEFINE([MAX_KEY], [1024], [Max length of the key in the INI file.]) + AC_CONFIG_FILES([Makefile ini_config.pc]) AC_OUTPUT diff --git a/common/ini/ini_config.c b/common/ini/ini_config.c index bd4fa79..6766dfa 100644 --- a/common/ini/ini_config.c +++ b/common/ini/ini_config.c @@ -110,11 +110,15 @@ const char *parsing_error_str(int parsing_error) return str_error[parsing_error-1]; } -/* Function to return grammar error */ +/* Function to return grammar error. + * This functions is currently not used. + * It is planned to be used by the INI + * file grammar parser. + */ const char *grammar_error_str(int grammar_error) { const char *placeholder= _("Unknown grammar error."); - /* FIXME - this is a temporary placeholder !!!! */ + /* THIS IS A TEMPORARY PLACEHOLDER !!!! */ const char *str_error[] = { _(""), _(""), _(""), @@ -131,11 +135,15 @@ const char *grammar_error_str(int grammar_error) return str_error[grammar_error-1]; } -/* Function to return validation error */ +/* Function to return validation error. + * This functions is currently not used. + * It is planned to be used by the INI + * file grammar validator. + */ const char *validation_error_str(int validation_error) { const char *placeholder= _("Unknown validation error."); - /* FIXME - this is a temporary placeholder !!!! */ + /* THIS IS A TEMPORARY PLACEHOLDER !!!! */ const char *str_error[] = { _(""), _(""), _(""), diff --git a/common/ini/ini_config.h b/common/ini/ini_config.h index 5ae4981..f7e0415 100644 --- a/common/ini/ini_config.h +++ b/common/ini/ini_config.h @@ -26,6 +26,7 @@ #include <limits.h> #include <stdio.h> #include "collection.h" +#include "config.h" /* Name of the default (missing section in the INI file */ #define INI_DEFAULT_SECTION "default" @@ -68,9 +69,7 @@ -/* Internal sizes */ -/* FIXME - make them configurable via config.h */ -#define MAX_KEY 1024 +/* Internal sizes. MAX_KEY is defined in config.h */ #define MAX_VALUE PATH_MAX #define BUFFER_SIZE MAX_KEY + MAX_VALUE + 3 -- 1.5.5.6
From b9408f914611826d23f8b03097bd9e07fbb0fc5e Mon Sep 17 00:00:00 2001 From: Dmitri Pal <[email protected]> Date: Fri, 4 Dec 2009 17:00:47 -0500 Subject: [PATCH] DHASH: Adding new function to interface This function allows deleting the hesh element without destroying the actual data in the hash. This is for "give me the data and remove the hash element" use case. --- common/dhash/dhash.c | 34 ++++++++++++++++++++++++++++++++-- common/dhash/dhash.h | 14 +++++++++++++- common/dhash/dhash_example.c | 11 +++++++++++ 3 files changed, 56 insertions(+), 3 deletions(-) diff --git a/common/dhash/dhash.c b/common/dhash/dhash.c index 92283a2..2530795 100644 --- a/common/dhash/dhash.c +++ b/common/dhash/dhash.c @@ -889,7 +889,8 @@ int hash_lookup(hash_table_t *table, hash_key_t *key, hash_value_t *value) } } -int hash_delete(hash_table_t *table, hash_key_t *key) + +int hash_remove(hash_table_t *table, hash_key_t *key, hash_value_t *value) { int error; segment_t element, *chain; @@ -902,7 +903,7 @@ int hash_delete(hash_table_t *table, hash_key_t *key) lookup(table, key, &element, &chain); if (element) { - if (table->delete_callback) table->delete_callback(&element->entry); + if (value) *value = element->entry.value; *chain = element->next; /* remove from chain */ /* * Table too sparse? @@ -920,4 +921,33 @@ int hash_delete(hash_table_t *table, hash_key_t *key) } } +int hash_delete(hash_table_t *table, hash_key_t *key) +{ + int error; + segment_t element, *chain; + + if (!table) return HASH_ERROR_BAD_TABLE; + + if (!is_valid_key_type(key->type)) + return HASH_ERROR_BAD_KEY_TYPE; + lookup(table, key, &element, &chain); + + if (element) { + if (table->delete_callback) table->delete_callback(&element->entry); + *chain = element->next; /* remove from chain */ + /* + * Table too sparse? + */ + if (--table->entry_count / table->bucket_count < table->min_load_factor) { + if ((error = contract_table(table)) != HASH_SUCCESS) { /* doesn't affect element */ + return error; + } + } + if (element->entry.key.type == HASH_KEY_STRING) table->free ((char *)element->entry.key.str); + table->free(element); + return HASH_SUCCESS; + } else { + return HASH_ERROR_KEY_NOT_FOUND; + } +} diff --git a/common/dhash/dhash.h b/common/dhash/dhash.h index 2bc5e6e..53589bb 100644 --- a/common/dhash/dhash.h +++ b/common/dhash/dhash.h @@ -244,13 +244,25 @@ int hash_get_default(hash_table_t *table, hash_key_t *key, hash_value_t *value, * Delete the item from the table. The key and its type are specified in the key * parameter which are passed by reference. If the key was in the table * HASH_SUCCESS is returned otherwise HASH_ERROR_KEY_NOT_FOUND is - * returned. Memory allocated to hold the key if it was a string is free by the + * returned. Memory allocated to hold the key if it was a string is freed by the * hash library, but values which are pointers to user data must be freed by the * caller (see delete_callback). */ int hash_delete(hash_table_t *table, hash_key_t *key); /* + * Delete the key from the table and return the value if value pointer is not NULL. + * If value ponter is NULL the value is not returned but also not destroyed. + * The key and its type are specified in the key parameter which are passed + * by reference. If the key was in the table HASH_SUCCESS is returned otherwise + * HASH_ERROR_KEY_NOT_FOUND is returned. Memory allocated to hold the key + * if it was a string is freed by the hash library, but values which are pointers + * to user data must be freed by the caller. + */ +int hash_remove(hash_table_t *table, hash_key_t *key, hash_value_t *value); + + +/* * Often it is useful to operate on every key and/or value in the hash * table. The hash_iterate function will invoke the users callback on every item * in the table as long as the callback returns a non-zero value. Returning a diff --git a/common/dhash/dhash_example.c b/common/dhash/dhash_example.c index b7de623..2875a2f 100644 --- a/common/dhash/dhash_example.c +++ b/common/dhash/dhash_example.c @@ -78,6 +78,17 @@ int main(int argc, char **argv) fprintf(stderr, "cannot find key \"%s\" (%s)\n", key.str, hash_error_string(error)); } + /* Get value back removing element from the table */ + if ((error = hash_remove(table, &key, &value)) != HASH_SUCCESS) { + fprintf(stderr, "cannot find and remove key \"%s\" (%s)\n", key.str, hash_error_string(error)); + } + + /* Add the element back */ + if ((error = hash_enter(table, &key, &value)) != HASH_SUCCESS) { + fprintf(stderr, "cannot add to table \"%s\" (%s)\n", key.str, hash_error_string(error)); + return error; + } + /* Visit each entry in the table, callback will increment count on each visit */ printf("Iterate using callback\n"); count = 0; -- 1.5.5.6
From c251a07316aa587c7856b2358360f9d8d24bcce7 Mon Sep 17 00:00:00 2001 From: Dmitri Pal <[email protected]> Date: Fri, 16 Oct 2009 23:23:38 -0400 Subject: [PATCH] COMMON Improvements to the trace macro Added more distingushable indication to the trace messages that represent errors. --- common/trace/trace.h | 24 +++++++++++++++--------- 1 files changed, 15 insertions(+), 9 deletions(-) diff --git a/common/trace/trace.h b/common/trace/trace.h index b2604fd..c7e375b 100644 --- a/common/trace/trace.h +++ b/common/trace/trace.h @@ -44,9 +44,11 @@ extern unsigned trace_level; #define TRACE_STRING(level, msg, str) \ do { \ if (level & trace_level) { \ - printf("[DEBUG] %23s (%4d) %s %s\n", \ - __FILE__, __LINE__, (msg != NULL) ? msg : "MISSING MESSAGE", \ - (str != NULL) ? str : "(null)"); \ + printf("[DEBUG] %40s (%4d) %s%s %s\n", \ + __FILE__, __LINE__, \ + (level == TRACE_ERROR) ? "ERROR-> " : "", \ + (msg != NULL) ? msg : "MISSING MESSAGE", \ + (str != NULL) ? str : "(null)"); \ } \ } while(0) @@ -54,9 +56,11 @@ extern unsigned trace_level; #define TRACE_NUMBER(level, msg, num) \ do { \ if (level & trace_level) { \ - printf("[DEBUG] %23s (%4d) %s %lu\n", \ - __FILE__, __LINE__, (msg != NULL) ? msg : "MISSING MESSAGE", \ - (unsigned long int)(num)); \ + printf("[DEBUG] %40s (%4d) %s%s %lu\n", \ + __FILE__, __LINE__, \ + (level == TRACE_ERROR) ? "ERROR-> " : "", \ + (msg != NULL) ? msg : "MISSING MESSAGE", \ + (unsigned long int)(num)); \ } \ } while(0) @@ -64,9 +68,11 @@ extern unsigned trace_level; #define TRACE_DOUBLE(level, msg, num) \ do { \ if (level & trace_level) { \ - printf("[DEBUG] %23s (%4d) %s %e\n", \ - __FILE__, __LINE__, (msg != NULL) ? msg : "MISSING MESSAGE", \ - (double)(num)); \ + printf("[DEBUG] %40s (%4d) %s%s %e\n", \ + __FILE__, __LINE__, \ + (level == TRACE_ERROR) ? "ERROR-> " : "", \ + (msg != NULL) ? msg : "MISSING MESSAGE", \ + (double)(num)); \ } \ } while(0) -- 1.5.5.6
From 4ae19c3198997b90f5b10e4a8ee2399dac357fcb Mon Sep 17 00:00:00 2001 From: Dmitri Pal <[email protected]> Date: Mon, 7 Dec 2009 09:14:10 -0500 Subject: [PATCH] COLLECTION: Cleaning FIXME comments I scanned through the code and made sure that the FIXME comments are either addressed or a corresponding ticket exists. I removed two comments that had "FIXME" in them. The tickets for those comments are #72 and #308. --- common/collection/collection.c | 1 - common/collection/collection.h | 19 ------------------- common/collection/configure.ac | 2 ++ 3 files changed, 2 insertions(+), 20 deletions(-) diff --git a/common/collection/collection.c b/common/collection/collection.c index c8a0077..e3644e4 100644 --- a/common/collection/collection.c +++ b/common/collection/collection.c @@ -2455,7 +2455,6 @@ int col_get_reference_from_item(struct collection_item *ci, /* ADDITION */ /* Add collection to collection */ -/* FIXME - allow to add collection to a collection with disposition */ int col_add_collection_to_collection(struct collection_item *ci, const char *sub_collection_name, const char *as_property, diff --git a/common/collection/collection.h b/common/collection/collection.h index 665d5f0..1121b16 100644 --- a/common/collection/collection.h +++ b/common/collection/collection.h @@ -51,10 +51,6 @@ collections */ -/* Any data we deal with can't be longer than this */ -/* FIXME - make it compile time option */ -#define COL_MAX_DATA 65535 - /* Default class for a free form collection */ #define COL_CLASS_DEFAULT 0 @@ -219,21 +215,6 @@ int col_create_collection(struct collection_item **ci, const char *name, unsigned cclass); -/* Function that creates a named collection using a memory descriptor */ -/* FIXME - function is a placeholder. It is not implemented yet. - * will be added in future together with the definition of the - * descriptor structure. - * The purpose is to control the internal implementation of the collection - * a) Use hash table for faster searches if the collection is expected to be large. - * b) Define memory functions to use. - */ -/* -int col_create_collection_ex(struct collection_item **ci, - const char *name, - unsigned cclass, - struct cdescriptor *descrptor); -*/ - /* Function that destroys a collection */ void col_destroy_collection(struct collection_item *ci); diff --git a/common/collection/configure.ac b/common/collection/configure.ac index cf7a1ff..3ba1eb7 100644 --- a/common/collection/configure.ac +++ b/common/collection/configure.ac @@ -30,6 +30,8 @@ AC_CHECK_FUNC([strcasestr], [Define if strcasestr exists]), AC_MSG_ERROR("Platform must support strcasestr")) +AC_DEFINE([COL_MAX_DATA], [65535], [Max length of the data block allowed in the collection value.]) + AC_CONFIG_FILES([Makefile collection.pc]) AC_OUTPUT -- 1.5.5.6
_______________________________________________ sssd-devel mailing list [email protected] https://fedorahosted.org/mailman/listinfo/sssd-devel
