rewrite cf_reference_item()

while it's now more code, the code is at least clearer, and also
returns descriptive errors sayng exactly what went wrong.

Update the callers to print out these errors

diff --git a/src/lib/server/cf_file.c b/src/lib/server/cf_file.c
index 12a3fc8d3f4ef3f55e2a6ed938f8cc9ee437a510..dd53ff463545314e4e9f3d94549abf88a2d66ab6 100644 (file)
--- a/src/lib/server/cf_file.c
+++ b/src/lib/server/cf_file.c
@@ -295,7 +295,7 @@ char const *cf_expand_variables(char const *cf, int lineno,
                        ci = cf_reference_item(parent_cs, outer_cs, name);
                        if (!ci) {
                                if (soft_fail) *soft_fail = true;
-                               ERROR("%s[%d]: Reference \"${%s}\" not found", cf, lineno, name);
+                               PERROR("%s[%d]: Failed finding reference \"${%s}\"", cf, lineno, name);
                                return NULL;
                        }
 
@@ -1215,8 +1215,8 @@ static int process_template(cf_stack_t *stack)
 
        ci = cf_reference_item(parent_cs, templatecs, stack->buff[2]);
        if (!ci || (ci->type != CONF_ITEM_SECTION)) {
-               ERROR("%s[%d]: No such template \"%s\" in the 'templates' section.",
-                     frame->filename, frame->lineno, stack->buff[2]);
+               PERROR("%s[%d]: Failed finding item \"%s\" in the 'templates' section.",
+                      frame->filename, frame->lineno, stack->buff[2]);
                return -1;
        }
 
@@ -3734,10 +3734,18 @@ CONF_ITEM *cf_reference_item(CONF_SECTION const *parent_cs,
        CONF_PAIR               *cp;
        CONF_SECTION            *next;
        CONF_SECTION const      *cs = outer_cs;
-       char                    name[8192];
-       char                    *p;
+       char                    name[8192], *p;
+       char const              *name2;
 
-       if (!ptr || (!parent_cs && !outer_cs)) return NULL;
+       if (!ptr || (!parent_cs && !outer_cs)) {
+               fr_strerror_const("Invalid argument");
+               return NULL;
+       }
+
+       if (!*ptr) {
+               fr_strerror_const("Empty string is invalid");
+               return NULL;
+       }
 
        strlcpy(name, ptr, sizeof(name));
 
@@ -3752,7 +3760,7 @@ CONF_ITEM *cf_reference_item(CONF_SECTION const *parent_cs,
                /*
                 *      Just '.' means the current section
                 */
-               if (*p == '\0') return cf_section_to_item(cs);
+               if (*p == '\0') return cf_section_to_item(cs); /* const issues */
 
                /*
                 *      ..foo means "foo from the section
@@ -3765,95 +3773,190 @@ CONF_ITEM *cf_reference_item(CONF_SECTION const *parent_cs,
                         *      .. means the section
                         *      enclosing this section
                         */
-                       if (!*++p) return cf_section_to_item(cs);
+                       if (!*++p) return cf_section_to_item(cs); /* const issues */
                }
 
                /*
-                *      "foo.bar.baz" means "from the root"
+                *      "foo.bar.baz" means "from the given root"
                 */
        } else if (strchr(p, '.') != NULL) {
-               if (!parent_cs) return NULL;
+               if (!parent_cs) {
+               missing_parent:
+                       fr_strerror_const("Missing parent configuration section");
+                       return NULL;
+               }
+               cs = parent_cs;
+
+               /*
+                *      "foo" could be from the current section, either as a
+                *      section or as a pair.
+                *
+                *      If that isn't found, search from the given root.
+                */
+       } else {
+               next = cf_section_find(cs, p, NULL);
+               if (!next && cs->template) next = cf_section_find(cs->template, p, NULL);
+               if (next) return &(next->item);
+
+               cp = cf_pair_find(cs, p);
+               if (!cp && cs->template) cp = cf_pair_find(cs->template, p);
+               if (cp) return &(cp->item);
+
+               if (!parent_cs) goto missing_parent;
                cs = parent_cs;
        }
 
+       /*
+        *      Chop the string into pieces, and look up the pieces.
+        */
        while (*p) {
-               char *q, *r;
+               char *n1, *n2, *q;
+
+               ERROR("Parent %s %s at %d - %s",
+                     cf_section_name1(cs), cf_section_name2(cs), __LINE__, p);
 
-               r = strchr(p, '[');
-               q = strchr(p, '.');
-               if (!r && !q) break;
+               n1 = p;
+               n2 = NULL;
+               q = p;
 
-               if (r && q > r) q = NULL;
-               if (q && q < r) r = NULL;
+               fr_assert(*q);
 
                /*
-                *      Split off name2.
+                *      Look for a terminating '.' or '[', to get name1 and possibly name2.
                 */
-               if (r) {
-                       q = strchr(r + 1, ']');
-                       if (!q) return NULL; /* parse error */
-
+               while (*q != '\0') {
                        /*
-                        *      Points to foo[bar]xx: parse error,
-                        *      it should be foo[bar] or foo[bar].baz
+                        *      foo.bar -> return "foo"
                         */
-                       if (q[1] && q[1] != '.') return NULL;
+                       if (*q == '.') {
+                               *q++ = '\0'; /* leave 'q' after the '.' */
+                               break;
+                       }
 
-                       *r = '\0';
-                       *q = '\0';
-                       next = cf_section_find(cs, p, r + 1);
-                       if (!next && cs->template) next = cf_section_find(cs->template, p, r + 1);
-                       *r = '[';
-                       *q = ']';
+                       /*
+                        *      name1 is anything up to '[' or EOS.
+                        */
+                       if (*q != '[') {
+                               q++;
+                               continue;
+                       }
 
                        /*
-                        *      Points to a named instance of a section.
+                        *      Found "name1[", look for "name2]" or "name2]."
                         */
-                       if (!q[1]) {
-                               if (!next) return NULL;
-                               return &(next->item);
+                       *q++ = '\0';
+                       n2 = q;
+
+                       while (*q != '\0') {
+                               if (*q == '[') {
+                                       fr_strerror_const("Invalid reference, '[' cannot be used inside of a '[...]' block");
+                                       return NULL;
+                               }
+
+                               if (*q != ']') {
+                                       q++;
+                                       continue;
+                               }
+
+                               /*
+                                *      We've found the trailing ']'
+                                */
+                               *q++ = '\0';
+
+                               /*
+                                *      "name2]"
+                                */
+                               if (!*q) break;
+
+                               /*
+                                *      Must be "name2]."
+                                */
+                               if (*q++ == '.') break;
+
+                               fr_strerror_const("Invalid reference, ']' is not followed by '.'");
+                               return NULL;
                        }
 
-                       q++;    /* ensure we skip the ']' and '.' */
+                       if (n2) break;
 
-               } else {
-                       *q = '\0';
-                       next = cf_section_find(cs, p, NULL);
-                       if (!next && cs->template) next = cf_section_find(cs->template, p, NULL);
-                       *q = '.';
+                       /*
+                        *      "name1[name2", but not "name1[name2]"
+                        */
+                       fr_strerror_printf("Invalid reference after '%s', missing close ']'", n2);
+                       return NULL;
                }
+               p = q;          /* get it ready for the next round */
 
-               if (!next) break; /* it MAY be a pair in this section! */
+               /*
+                *      End of the string.  The thing could be a section with
+                *      two names, a section with one name, or a pair.
+                *
+                *      And if we don't find the thing we're looking for here,
+                *      check the template section.
+                */
+               if (!*p) {
+                       /*
+                        *      Two names, must be a section.
+                        */
+                       if (n2) {
+                               next = cf_section_find(cs, n1, n2);
+                               if (!next && cs->template) next = cf_section_find(cs->template, n1, n2);
+                               if (next) return &(next->item);
+
+                       fail:
+                               name2 = cf_section_name2(cs);
+                               fr_strerror_printf("Parent section %s%s%s { ... } does not contain a %s %s { ... } configuration section",
+                                                  cf_section_name1(cs),
+                                                  name2 ? " " : "", name2 ? name2 : "",
+                                                  n1, n2);
+                               return NULL;
+                       }
 
-               cs = next;
-               p = q + 1;
-       }
+                       /*
+                        *      One name, the final thing can be a section or a pair.
+                        */
+                       next = cf_section_find(cs, n1, NULL);
+                       if (!next && cs->template) next = cf_section_find(cs->template, n1, NULL);
 
-       if (!*p) return NULL;
+                       if (next) return &(next->item);
 
-retry:
-       /*
-        *      Find it in the current referenced
-        *      section.
-        */
-       cp = cf_pair_find(cs, p);
-       if (!cp && cs->template) cp = cf_pair_find(cs->template, p);
-       if (cp) {
-               cp->referenced = true;  /* conf pairs which are referenced count as used */
-               return &(cp->item);
-       }
+                       cp = cf_pair_find(cs, n1);
+                       if (!cp && cs->template) cp = cf_pair_find(cs->template, n1);
+                       if (cp) return &(cp->item);
 
-       next = cf_section_find(cs, p, NULL);
-       if (next) return &(next->item);
+                       name2 = cf_section_name2(cs);
+                       fr_strerror_printf("Parent section %s%s%s  { ... } does not contain a %s configuration item",
+                                          cf_section_name1(cs),
+                                          name2 ? " " : "", name2 ? name2 : "",
+                                          n1);
+                       return NULL;
+               }
 
-       /*
-        *      "foo" is "in the current section, OR in main".
-        */
-       if ((p == name) && (parent_cs != NULL) && (cs != parent_cs)) {
-               cs = parent_cs;
-               goto retry;
+               /*
+                *      There's more to the string.  The thing we're looking
+                *      for MUST be a configuration section.
+                */
+               next = cf_section_find(cs, n1, n2);
+               if (!next && cs->template) next = cf_section_find(cs->template, n1, n2);
+               if (next) {
+                       cs = next;
+                       continue;
+               }
+
+               if (n2) goto fail;
+
+               name2 = cf_section_name2(cs);
+               fr_strerror_printf("Parent section %s%s%s { ... } does not contain a %s { ... } configuration section",
+                                  cf_section_name1(cs),
+                                  name2 ? " " : "", name2 ? name2 : "",
+                                  n1);
+               return NULL;
        }
 
+       /*
+        *      We've fallen off of the end of the string.  This should not have happened!
+        */
+       fr_strerror_const("Cannot parse reference");
        return NULL;
 }
 

diff --git a/src/lib/server/main_config.c b/src/lib/server/main_config.c
index afe06ee91adcabd7a80b26721a2a993a0b510a80..62becaa5f0ec0cc740d3a21fd455609fe1068b93 100644 (file)
--- a/src/lib/server/main_config.c
+++ b/src/lib/server/main_config.c
@@ -576,7 +576,7 @@ static xlat_action_t xlat_config(TALLOC_CTX *ctx, fr_dcursor_t *out,
 
        ci = cf_reference_item(main_config->root_cs, main_config->root_cs, in_head->vb_strvalue);
        if (!ci || !cf_item_is_pair(ci)) {
-               REDEBUG("Config item \"%pV\" does not exist", in_head);
+               RPEDEBUG("Failed finding configuration item '%s'", in_head->vb_strvalue);
                return XLAT_ACTION_FAIL;
        }
 

diff --git a/src/lib/server/trigger.c b/src/lib/server/trigger.c
index e16037bd13bff2cea22e2d079b25dbdaa24f9d5c..141438ec49e714367b5b5d3db46a236da31ea641 100644 (file)
--- a/src/lib/server/trigger.c
+++ b/src/lib/server/trigger.c
@@ -277,7 +277,7 @@ int trigger_exec(unlang_interpret_t *intp,
 
        ci = cf_reference_item(subcs, trigger_exec_main, attr);
        if (!ci) {
-               DEBUG3("No trigger configured for: %s", attr);
+               DEBUG3("Failed finding trigger '%s'", attr);
                return -1;
        }
 

diff --git a/src/lib/unlang/compile.c b/src/lib/unlang/compile.c
index 5027c28807a6c29aba733831d33f9442e1d381c3..9e52ac0f7c4a945adc26230fe1812b28a724cece 100644 (file)
--- a/src/lib/unlang/compile.c
+++ b/src/lib/unlang/compile.c
@@ -1833,9 +1833,14 @@ bool unlang_compile_actions(unlang_mod_actions_t *actions, CONF_SECTION *action_
                        CONF_ITEM *subci;
                        char const *value = cf_pair_value(cp);
 
+                       if (!value) {
+                               cf_log_err(csi, "Missing reference string");
+                               return false;
+                       }
+
                        subci = cf_reference_item(cs, cf_root(cf_section_to_item(action_cs)), value);
                        if (!subci) {
-                               cf_log_err(csi, "Unknown reference '%s'", value ? value : "<INVALID>");
+                               cf_log_perr(csi, "Failed finding reference '%s'", value);
                                return false;
                        }
 

diff --git a/src/modules/rlm_linelog/rlm_linelog.c b/src/modules/rlm_linelog/rlm_linelog.c
index 6638df15e08f7d97f11fd2a8059b4434fdf64216..37075e901c66f074cbf0c6e8eaf2c3da4c86d279 100644 (file)
--- a/src/modules/rlm_linelog/rlm_linelog.c
+++ b/src/modules/rlm_linelog/rlm_linelog.c
@@ -734,7 +734,7 @@ static unlang_action_t CC_HINT(nonnull) mod_do_linelog(rlm_rcode_t *p_result, mo
 
                ci = cf_reference_item(NULL, inst->cs, p);
                if (!ci) {
-                       RDEBUG2("Path \"%s\" doesn't exist", p);
+                       RPDEBUG2("Failed finding reference '%s'", p);
                        goto default_msg;
                }