Hi list,

I got recently across an error a user claimed with the following output:

2008-07-23 06:16:16.258 [1887] [0] ERROR: Group 'pgsql-conneciton' may not contain field 'group'. 2008-07-23 06:16:16.258 [1887] [0] ERROR: Error found on line 377 of file `etc/kannel_smppbox.conf'. 2008-07-23 06:16:16.258 [1887] [0] PANIC: Error reading configuration file, cannot start.

and the first error line indicated that there was a 'group' directive "inside" of the pgsql-connection group. I searched almost 30 minutes and couldn't find any obvious config file error. Then started debugging in deeper. Then my eyes got the catch, it's a simple typo in the group identifier. It's 'pgsql-connection' and not the scrambled one.

Now, this seems to misleading users when parsing gets such errors, since they don't see any specific error that there is _no_ group identifier with that name.

I patched gwlib/cfg.c to provide such an error state if there is a group inside the config that out config parser doesn't know.

Actually it's fairly simple: the foobar_is_allowed_in_group() have to exit with -1 to indicate an unknown identifier and the allowed_hooks loop has to mangle a bit with previous and present result values of those hook returns. That's it.

(the rest is simply line indent fixes with NLC.)

Please test and vote for committing to CVS.

Stipe

-------------------------------------------------------------------
Kölner Landstrasse 419
40589 Düsseldorf, NRW, Germany

tolj.org system architecture      Kannel Software Foundation (KSF)
http://www.tolj.org/              http://www.kannel.org/

mailto:st_{at}_tolj.org           mailto:stolj_{at}_kannel.org
-------------------------------------------------------------------
### Eclipse Workspace Patch 1.0
#P gateway-cvs-head
Index: gwlib/cfg.c
===================================================================
RCS file: /home/cvs/gateway/gwlib/cfg.c,v
retrieving revision 1.39
diff -u -r1.39 cfg.c
--- gwlib/cfg.c 9 Jan 2008 20:06:56 -0000       1.39
+++ gwlib/cfg.c 23 Jul 2008 18:58:00 -0000
@@ -186,7 +186,8 @@
        }
     #include "cfg.def"
 
-    return 0;
+    /* unknown group identifier */
+    return -1;
 }
 
 
@@ -207,14 +208,15 @@
 static int is_allowed_in_group(Octstr *group, Octstr *variable)
 {
     long i;
-    int r = 0;
+    int x, r = -1;
 
     for (i = 0; i < gwlist_len(allowed_hooks); ++i) {
-        r += ((int(*)(Octstr *, Octstr *))
+        x = ((int(*)(Octstr *, Octstr *))
             gwlist_get(allowed_hooks, i))(group, variable);
+        r = (x == -1 ? (r == -1 ? x : r) : (r == -1 ? x : r + x));
     }
 
-    return (r > 0);
+    return r;
 }
 
 
@@ -248,34 +250,48 @@
     
     groupname = cfg_get(grp, octstr_imm("group"));
     if (groupname == NULL) {
-       error(0, "Group does not contain variable 'group'.");
-       return -1;
+        error(0, "Group does not contain variable 'group'.");
+        return -1;
     }
     set_group_name(grp, groupname);
 
     names = dict_keys(grp->vars);
+
     while ((name = gwlist_extract_first(names)) != NULL) {
-       if (!is_allowed_in_group(groupname, name)) {
-           error(0, "Group '%s' may not contain field '%s'.",
-                 octstr_get_cstr(groupname), octstr_get_cstr(name));
-           octstr_destroy(name);
-           octstr_destroy(groupname);
-           gwlist_destroy(names, octstr_destroy_item);
-           return -1;
-       }
-       octstr_destroy(name);
+        int a = is_allowed_in_group(groupname, name);
+        switch (a) {
+            case 0:
+                error(0, "Group '%s' may not contain field '%s'.",
+                      octstr_get_cstr(groupname), octstr_get_cstr(name));
+                octstr_destroy(name);
+                octstr_destroy(groupname);
+                gwlist_destroy(names, octstr_destroy_item);
+                return -1;
+                break;
+            case -1:
+                error(0, "Group '%s' is no valid group identifier.",
+                      octstr_get_cstr(groupname));
+                octstr_destroy(name);
+                octstr_destroy(groupname);
+                gwlist_destroy(names, octstr_destroy_item);
+                return -1;
+                break;
+            default:
+                octstr_destroy(name);
+                break;
+        }
     }
     gwlist_destroy(names, NULL);
 
-    if (is_single_group(groupname))
-       dict_put(cfg->single_groups, groupname, grp);
-    else {
-       list = dict_get(cfg->multi_groups, groupname);
-       if (list == NULL) {
-           list = gwlist_create();
-           dict_put(cfg->multi_groups, groupname, list);
-       }
-       gwlist_append(list, grp);
+    if (is_single_group(groupname)) {
+        dict_put(cfg->single_groups, groupname, grp);
+    } else {
+        list = dict_get(cfg->multi_groups, groupname);
+        if (list == NULL) {
+            list = gwlist_create();
+            dict_put(cfg->multi_groups, groupname, list);
+        }
+        gwlist_append(list, grp);
     }
 
     octstr_destroy(groupname);
@@ -564,7 +580,8 @@
                     grp = create_group(); 
                  
                 if (grp->configfile != NULL) {
-                    octstr_destroy(grp->configfile); grp->configfile=NULL;
+                    octstr_destroy(grp->configfile); 
+                    grp->configfile = NULL;
                 }
                 grp->configfile = octstr_duplicate(cfg->filename); 
 

Reply via email to