diff --git a/CHANGELOG.md b/CHANGELOG.md index e51816e8a..878908c20 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,7 +33,10 @@ * CLICON_XML_CHANGELOG enables the yang changelog feature * CLICON_XML_CHANGELOG_FILE where the changelog resides + ### API changes on existing features (you may need to change your code) +* Changed hash API for better error handling + * hash_dump, hash_keys, clicon_option_dump have new signatures * Renamed `xml_insert` to `xml_wrap_all`. * Added modules-state diff parameter to xmldb_get datastore function for startup scenarios. Set this to NULL in normal cases. * `rpc_callback_register` added a namespace parameter. Example: diff --git a/apps/backend/backend_main.c b/apps/backend/backend_main.c index 16141a749..f7aa12477 100644 --- a/apps/backend/backend_main.c +++ b/apps/backend/backend_main.c @@ -509,7 +509,9 @@ main(int argc, argc -= optind; argv += optind; + /* Access the remaining argv/argc options (after --) w clicon-argv_get() */ clicon_argv_set(h, argv0, argc, argv); + clicon_log_init(__PROGRAM__, debug?LOG_DEBUG:LOG_INFO, logdst); /* Defer: Wait to the last minute to print help message */ diff --git a/apps/cli/cli_main.c b/apps/cli/cli_main.c index 3e8f413ed..cfdabae0d 100644 --- a/apps/cli/cli_main.c +++ b/apps/cli/cli_main.c @@ -439,6 +439,9 @@ main(int argc, char **argv) argc -= optind; argv += optind; + /* Access the remaining argv/argc options (after --) w clicon-argv_get() */ + clicon_argv_set(h, argv0, argc, argv); + /* Defer: Wait to the last minute to print help message */ if (help) usage(h, argv[0]); diff --git a/apps/netconf/netconf_main.c b/apps/netconf/netconf_main.c index 7809c4d0b..fd0564114 100644 --- a/apps/netconf/netconf_main.c +++ b/apps/netconf/netconf_main.c @@ -472,6 +472,9 @@ main(int argc, argc -= optind; argv += optind; + /* Access the remaining argv/argc options (after --) w clicon-argv_get() */ + clicon_argv_set(h, argv0, argc, argv); + /* Create top-level yang spec and store as option */ if ((yspec = yspec_new()) == NULL) goto done; diff --git a/apps/restconf/restconf_main.c b/apps/restconf/restconf_main.c index dc21b5aa9..b4f521412 100644 --- a/apps/restconf/restconf_main.c +++ b/apps/restconf/restconf_main.c @@ -633,6 +633,9 @@ main(int argc, argc -= optind; argv += optind; + /* Access the remaining argv/argc options (after --) w clicon-argv_get() */ + clicon_argv_set(h, argv0, argc, argv); + /* Initialize plugins group */ if ((dir = clicon_restconf_dir(h)) != NULL) if (clixon_plugins_load(h, CLIXON_PLUGIN_INIT, dir, NULL) < 0) diff --git a/datastore/clixon_xmldb_text.c b/datastore/clixon_xmldb_text.c index 8477912b0..8273d2b58 100644 --- a/datastore/clixon_xmldb_text.c +++ b/datastore/clixon_xmldb_text.c @@ -201,7 +201,8 @@ text_disconnect(xmldb_handle xh) free(th->th_dbdir); if (th->th_dbs){ if (th->th_cache){ - keys = hash_keys(th->th_dbs, &klen); + if (hash_keys(th->th_dbs, &keys, &klen) < 0) + goto done; for(i = 0; i < klen; i++) if ((de = hash_value(th->th_dbs, keys[i], NULL)) != NULL){ if (de->de_xml) @@ -219,7 +220,7 @@ text_disconnect(xmldb_handle xh) free(th); } retval = 0; - // done: + done: return retval; } @@ -1605,23 +1606,26 @@ int text_unlock_all(xmldb_handle xh, int pid) { + int retval = -1; struct text_handle *th = handle(xh); char **keys = NULL; size_t klen; int i; struct db_element *de; - if ((keys = hash_keys(th->th_dbs, &klen)) == NULL) - return 0; + if (hash_keys(th->th_dbs, &keys, &klen) < 0) + goto done; for(i = 0; i < klen; i++) if ((de = hash_value(th->th_dbs, keys[i], NULL)) != NULL && de->de_pid == pid){ de->de_pid = 0; hash_add(th->th_dbs, keys[i], de, sizeof(*de)); } + retval = 0; + done: if (keys) free(keys); - return 0; + return retval; } /*! Check if database is locked diff --git a/lib/clixon/clixon_hash.h b/lib/clixon/clixon_hash.h index 0cbb8129c..090af5424 100644 --- a/lib/clixon/clixon_hash.h +++ b/lib/clixon/clixon_hash.h @@ -50,9 +50,8 @@ clicon_hash_t hash_lookup (clicon_hash_t *head, const char *key); void *hash_value (clicon_hash_t *head, const char *key, size_t *vlen); clicon_hash_t hash_add (clicon_hash_t *head, const char *key, void *val, size_t vlen); int hash_del (clicon_hash_t *head, const char *key); -void hash_dump(clicon_hash_t *head, FILE *f); -char **hash_keys(clicon_hash_t *hash, size_t *nkeys); - +int hash_dump(clicon_hash_t *head, FILE *f); +int hash_keys(clicon_hash_t *hash, char ***vector, size_t *nkeys); /* * Macros to iterate over hash contents. diff --git a/lib/clixon/clixon_options.h b/lib/clixon/clixon_options.h index 8abf3b2ac..922121ae3 100644 --- a/lib/clixon/clixon_options.h +++ b/lib/clixon/clixon_options.h @@ -79,7 +79,7 @@ enum startup_mode_t{ */ /* Print registry on file. For debugging. */ -void clicon_option_dump(clicon_handle h, int dblevel); +int clicon_option_dump(clicon_handle h, int dblevel); /* Add a clicon options overriding file setting */ int clicon_option_add(clicon_handle h, char *name, char *value); diff --git a/lib/src/clixon_hash.c b/lib/src/clixon_hash.c index 69f5fce22..fa47bfcc8 100644 --- a/lib/src/clixon_hash.c +++ b/lib/src/clixon_hash.c @@ -105,16 +105,16 @@ hash_bucket(const char *str) while(*str) n += (uint32_t)*str++; - return n % HASH_SIZE; } /*! Initialize hash table. * - * @retval Pointer to new hash table. + * @retval hash Pointer to new hash table. + * @retval NULL Error */ clicon_hash_t * -hash_init (void) +hash_init(void) { clicon_hash_t *hash; @@ -148,19 +148,18 @@ hash_free(clicon_hash_t *hash) free(hash); } - /*! Find hash key. * * @param[in] hash Hash table * @param[in] key Variable name * @retval variable Hash variable structure on success - * @retval NULL Error + * @retval NULL Not found */ clicon_hash_t hash_lookup(clicon_hash_t *hash, const char *key) { - uint32_t bkt; + uint32_t bkt; clicon_hash_t h; bkt = hash_bucket(key); @@ -176,11 +175,11 @@ hash_lookup(clicon_hash_t *hash, } /*! Get value of hash - * @param[in] hash Hash table - * @param[in] key Variable name - * @param[out] vlen Length of value (as returned by function) - * @retval value Hash value, length given in vlen - * @retval NULL Error + * @param[in] hash Hash table + * @param[in] key Variable name + * @param[out] vlen Length of value (as returned by function if != NULL) + * @retval value Hash value, length given in vlen + * @retval NULL Key not found or value NULL */ void * hash_value(clicon_hash_t *hash, @@ -191,7 +190,7 @@ hash_value(clicon_hash_t *hash, h = hash_lookup(hash, key); if (h == NULL) - return NULL; + return NULL; /* OK, key not found */ if (vlen) *vlen = h->h_vlen; @@ -204,8 +203,9 @@ hash_value(clicon_hash_t *hash, * @param[in] key Variable name * @param[in] val Variable value (pointer to) * @param[in] vlen Length of variable value - * @retval variable New hash structure on success - * @retval NULL Failure + * @retval hash New hash structure on success + * @retval NULL Error + * @note special case val is NULL and vlen==0 */ clicon_hash_t hash_add(clicon_hash_t *hash, @@ -213,10 +213,16 @@ hash_add(clicon_hash_t *hash, void *val, size_t vlen) { - void *newval; + void *newval = NULL; clicon_hash_t h; clicon_hash_t new = NULL; + /* Check NULL case */ + if ((val == NULL && vlen != 0) || + (val != NULL && vlen == 0)){ + clicon_err(OE_UNIX, EINVAL, "Mismatch in value and length, only one is zero"); + goto catch; + } /* If variable exist, don't allocate a new. just replace value */ h = hash_lookup(hash, key); if (h == NULL) { @@ -235,13 +241,15 @@ hash_add(clicon_hash_t *hash, h = new; } - /* Make copy of value. aligned */ - newval = malloc(align4(vlen+3)); - if (newval == NULL){ - clicon_err(OE_UNIX, errno, "malloc: %s", strerror(errno)); - goto catch; + if (vlen){ + /* Make copy of value. aligned */ + newval = malloc(align4(vlen+3)); + if (newval == NULL){ + clicon_err(OE_UNIX, errno, "malloc: %s", strerror(errno)); + goto catch; + } + memcpy(newval, val, vlen); } - memcpy(newval, val, vlen); /* Free old value if existing variable */ if (h->h_val) @@ -270,8 +278,8 @@ hash_add(clicon_hash_t *hash, * @param[in] hash Hash table * @param[in] key Variable name * - * @retval 0 success - * @retval -1 failure + * @retval 0 OK + * @retval -1 Key not found */ int hash_del(clicon_hash_t *hash, @@ -295,18 +303,22 @@ hash_del(clicon_hash_t *hash, /*! Return vector of keys in hash table * * @param[in] hash Hash table + * @param[out] vector Vector of keys, NULL if not found * @param[out] nkeys Size of key vector - * @retval vector Vector of keys - * @retval NULL Error + * @retval 0 OK + * @retval -1 Error + * @note: vector needs to be deallocated with free */ -char ** +int hash_keys(clicon_hash_t *hash, + char ***vector, size_t *nkeys) { - int bkt; + int retval = -1; + int bkt; clicon_hash_t h; - char **tmp; - char **keys = NULL; + char **tmp; + char **keys = NULL; *nkeys = 0; for (bkt = 0; bkt < HASH_SIZE; bkt++) { @@ -325,40 +337,48 @@ hash_keys(clicon_hash_t *hash, h = NEXTQ(clicon_hash_t, h); } while (h != hash[bkt]); } - - return keys; - + if (vector){ + *vector = keys; + keys = NULL; + } + retval = 0; catch: if (keys) free(keys); - return NULL; + return retval; } /*! Dump contents of hash to FILE pointer. * * @param[in] hash Hash structure * @param[in] f FILE pointer for print output - * @retval void + * @retval 0 OK + * @retval -1 Error */ -void +int hash_dump(clicon_hash_t *hash, FILE *f) { - int i; - char **keys; - void *val; + int retval = -1; + int i; + char **keys = NULL; + void *val; size_t klen; size_t vlen; if (hash == NULL) - return; - keys = hash_keys(hash, &klen); - if (keys == NULL) - return; - + goto ok; + if (hash_keys(hash, &keys, &klen) < 0) + goto done; for(i = 0; i < klen; i++) { val = hash_value(hash, keys[i], &vlen); printf("%s =\t 0x%p , length %zu\n", keys[i], val, vlen); } - free(keys); + + ok: + retval = 0; + done: + if (keys) + free(keys); + return retval; } diff --git a/lib/src/clixon_options.c b/lib/src/clixon_options.c index e16f97ffa..456b38a3b 100644 --- a/lib/src/clixon_options.c +++ b/lib/src/clixon_options.c @@ -86,24 +86,25 @@ static const map_str2int startup_mode_map[] = { }; /*! Print registry on file. For debugging. + * @param[in] h Clicon handle + * @param[in] dbglevel Debug level + * @retval 0 OK + * @retval -1 Error */ -void +int clicon_option_dump(clicon_handle h, int dbglevel) { + int retval = -1; clicon_hash_t *hash = clicon_options(h); int i; - char **keys; + char **keys = NULL; void *val; size_t klen; size_t vlen; - if (hash == NULL) - return; - keys = hash_keys(hash, &klen); - if (keys == NULL) - return; - + if (hash_keys(hash, &keys, &klen) < 0) + goto done; for(i = 0; i < klen; i++) { val = hash_value(hash, keys[i], &vlen); if (vlen){ @@ -115,7 +116,11 @@ clicon_option_dump(clicon_handle h, else clicon_debug(dbglevel, "%s = NULL", keys[i]); } - free(keys); + retval = 0; + done: + if (keys) + free(keys); + return retval; } /*! Read filename and set values to global options registry. XML variant. @@ -1063,12 +1068,16 @@ clicon_argv_get(clicon_handle h, clicon_hash_t *cdat = clicon_data(h); void *p; - if ((p = hash_value(cdat, "argc", NULL)) == NULL) - return -1; - *argc = *(int*)p; - if ((p = hash_value(cdat, "argv", NULL)) == NULL) - return -1; - *argv = *(char***)p; + if (argc){ + if ((p = hash_value(cdat, "argc", NULL)) == NULL) + return -1; + *argc = *(int*)p; + } + if (argv){ + if ((p = hash_value(cdat, "argv", NULL)) == NULL) + return -1; + *argv = *(char***)p; + } return 0; } @@ -1076,9 +1085,10 @@ clicon_argv_get(clicon_handle h, * @param[in] h Clicon handle * @param[in] prog argv[0] - the program name * @param[in] argc Length of argv - * @param[in] argv Array of command-line options + * @param[in] argv Array of command-line options or NULL * @retval 0 OK * @retval -1 Error + * @note If argv=NULL deallocate allocated argv vector if exists. */ int clicon_argv_set(clicon_handle h, @@ -1086,20 +1096,25 @@ clicon_argv_set(clicon_handle h, int argc, char **argv) { + int retval = -1; clicon_hash_t *cdat = clicon_data(h); char **argvv = NULL; /* add space for null-termination and argv[0] program name */ if ((argvv = calloc(argc+2, sizeof(char*))) == NULL){ clicon_err(OE_UNIX, errno, "calloc"); - return -1; + goto done; } memcpy(argvv+1, argv, argc*sizeof(char*)); argvv[0] = prgm; if (hash_add(cdat, "argv", &argvv, sizeof(argvv))==NULL) - return -1; + goto done; argc += 1; if (hash_add(cdat, "argc", &argc, sizeof(argc))==NULL) - return -1; - return 0; + goto done; + retval = 0; + done: + if (argvv) + free(argvv); + return retval; }