Changed hash API for better error handling

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:

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 */

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]);

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;

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)

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)
+    if (hash_keys(th->th_dbs, &keys, &klen) < 0)
-	return 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

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);
+int hash_dump(clicon_hash_t *head, FILE *f);
-char **hash_keys(clicon_hash_t *hash, size_t *nkeys);
+int hash_keys(clicon_hash_t *hash, char ***vector, size_t *nkeys);
-
 
 /*
  *   Macros to iterate over hash contents.

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);

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,13 +148,12 @@ 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, 
@@ -178,9 +177,9 @@ 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)
+ * @param[out] vlen   Length of value (as returned by function if != NULL)
  * @retval     value  Hash value, length given in vlen 
- * @retval NULL     Error
+ * @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    hash   New hash structure on success
- * @retval NULL      Failure
+ * @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,6 +241,7 @@ hash_add(clicon_hash_t *hash,
 	h = new;
     }
     
+    if (vlen){
 	/* Make copy of value. aligned */
 	newval = malloc(align4(vlen+3)); 
 	if (newval == NULL){
@@ -242,6 +249,7 @@ hash_add(clicon_hash_t *hash,
 	    goto catch;
 	}
 	memcpy(newval, val, vlen);
+    }
     
     /* Free old value if existing variable */
     if (h->h_val)
@@ -270,8 +278,8 @@ catch:
  * @param[in] hash    Hash table
  * @param[in] key     Variable name
  *
- * @retval   0  success
+ * @retval    0       OK
- * @retval  -1  failure
+ * @retval   -1       Key not found
  */
 int
 hash_del(clicon_hash_t *hash, 
@@ -295,14 +303,18 @@ 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      0       OK
- * @retval      NULL    Error
+ * @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           retval = -1;
     int           bkt;
     clicon_hash_t h;
     char        **tmp;
@@ -325,40 +337,48 @@ hash_keys(clicon_hash_t *hash,
 	    h = NEXTQ(clicon_hash_t, h);
 	} while (h != hash[bkt]);
     }
-
+    if (vector){
-    return keys;
+	*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    retval = -1;
     int    i;
-    char **keys;
+    char **keys = NULL;
     void  *val;
     size_t klen;
     size_t vlen;
     
     if (hash == NULL)
-	return;
+	goto ok;
-    keys = hash_keys(hash, &klen);
+    if (hash_keys(hash, &keys, &klen) < 0)
-    if (keys == NULL)
+	goto done;
-	return;
-	
     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);
     }
+
+ ok:
+    retval = 0;
+ done:
+    if (keys)
 	free(keys);
+    return retval;
 }

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)
+    if (hash_keys(hash, &keys, &klen) < 0)
-	return;
+	goto done;
-    keys = hash_keys(hash, &klen);
-    if (keys == NULL)
-	return;
-	
     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]);
     }
+    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 (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;
+	goto done;
-    return 0;
+    retval = 0;
+ done:
+    if (argvv)
+	free(argvv);
+    return retval;
 }