You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by pq...@apache.org on 2004/09/03 04:31:06 UTC
cvs commit: httpd-2.0/modules/generators mod_info.c
pquerna 2004/09/02 19:31:06
Modified: . CHANGES
modules/generators mod_info.c
Log:
Rewrote config tree walk using recursion the way God intended.
Added ?config option. Added printout of config filename and line numbers.
PR: 30919
Submitted by: Rici Lake <rici ricilake.net>
Revision Changes Path
1.1584 +4 -0 httpd-2.0/CHANGES
Index: CHANGES
===================================================================
RCS file: /home/cvs/httpd-2.0/CHANGES,v
retrieving revision 1.1583
retrieving revision 1.1584
diff -u -r1.1583 -r1.1584
--- CHANGES 2 Sep 2004 19:49:19 -0000 1.1583
+++ CHANGES 3 Sep 2004 02:31:05 -0000 1.1584
@@ -2,6 +2,10 @@
[Remove entries to the current 2.0 section below, when backported]
+ *) mod_info: Rewrote config tree walk using a recursive function.
+ Added ?config option. Added printout of config filename and line numbers.
+ [Rici Lake <rici ricilake.net>, Paul Querna]
+
*) mod_proxy: Fix type error that prevents proxy-sendchunks from working.
[Justin Erenkrantz]
1.57 +98 -95 httpd-2.0/modules/generators/mod_info.c
Index: mod_info.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/generators/mod_info.c,v
retrieving revision 1.56
retrieving revision 1.57
diff -u -r1.56 -r1.57
--- mod_info.c 9 Feb 2004 20:29:19 -0000 1.56
+++ mod_info.c 3 Sep 2004 02:31:06 -0000 1.57
@@ -25,6 +25,7 @@
* GET /server-info?server - Returns server configuration only
* GET /server-info?module_name - Returns configuration for a single module
* GET /server-info?list - Returns quick list of included modules
+ * GET /server-info?config - Returns full configuration
*
* Rasmus Lerdorf <ra...@vex.net>, May 1996
*
@@ -39,6 +40,12 @@
* 8.11.00 Port to Apache 2.0. Read configuation from the configuration
* tree rather than reparse the entire configuation file.
*
+ * Rici Lake <ri...@ricilake.net>
+ *
+ * 2004-08-28 Rewrote config tree walk using recursion the way God intended.
+ * Added ?config option. Added printout of config filename and line numbers.
+ * Fixed indentation.
+ *
*/
#define CORE_PRIVATE
@@ -86,108 +93,97 @@
return new;
}
-static void mod_info_html_cmd_string(request_rec *r, const char *string,
- int close)
+static void mod_info_indent(request_rec * r, int nest, const char* thisfn, int linenum)
{
- const char *s;
-
- s = string;
- /* keep space for \0 byte */
- while (*s) {
- if (*s == '<') {
- if (close) {
- ap_rputs("</", r);
- } else {
- ap_rputs("<", r);
- }
- }
- else if (*s == '>') {
- ap_rputs(">", r);
- }
- else if (*s == '&') {
- ap_rputs("&", r);
- }
- else if (*s == ' ') {
- if (close) {
- ap_rputs(">", r);
- break;
- } else {
- ap_rputc(*s, r);
- }
- } else {
- ap_rputc(*s, r);
- }
- s++;
+ int i;
+ const char *prevfn = ap_get_module_config(r->request_config, &info_module);
+ char buf[32];
+ if (thisfn == NULL) thisfn = "*UNKNOWN*";
+ if (prevfn == NULL || 0 != strcmp(prevfn, thisfn)) {
+ thisfn = ap_escape_html(r->pool, thisfn);
+ ap_rprintf(r, "<dd><tt><strong>In file: %s</strong></tt></dd>\n", thisfn);
+ ap_set_module_config(r->request_config, &info_module, thisfn);
}
+ ap_rputs("<dd><tt>", r);
+ if (linenum > 0) sprintf(buf, "%d", linenum);
+ else buf[0] = '\0';
+ for (i = strlen(buf); i < 4; ++i) ap_rputs(" ", r);
+ ap_rputs(buf, r);
+ ap_rputs(": ", r);
+ for (i = 1; i <= nest; ++i) ap_rputs(" ", r);
}
-static void mod_info_module_cmds(request_rec * r, const command_rec * cmds,
- ap_directive_t * conftree)
+static void mod_info_show_cmd(request_rec * r, const ap_directive_t * dir,
+ int nest)
{
- const command_rec *cmd;
- ap_directive_t *tmptree = conftree;
- char htmlstring[MAX_STRING_LEN];
- int block_start = 0;
- int nest = 0;
-
- while (tmptree != NULL) {
- cmd = cmds;
- while (cmd->name) {
- if ((cmd->name[0] != '<') &&
- (strcasecmp(cmd->name, tmptree->directive) == 0)) {
- if (nest > block_start) {
- block_start++;
- apr_snprintf(htmlstring, sizeof(htmlstring), "%s %s",
- tmptree->parent->directive,
- tmptree->parent->args);
- ap_rputs("<dd><tt>", r);
- mod_info_html_cmd_string(r, htmlstring, 0);
- ap_rputs("</tt></dd>\n", r);
- }
- if (nest == 2) {
- ap_rprintf(r, "<dd><tt> %s "
- "<i>%s</i></tt></dd>\n",
- ap_escape_html(r->pool,tmptree->directive),
- ap_escape_html(r->pool,tmptree->args));
- } else if (nest == 1) {
- ap_rprintf(r,
- "<dd><tt> %s <i>%s</i></tt></dd>\n",
- ap_escape_html(r->pool,tmptree->directive),
- ap_escape_html(r->pool,tmptree->args));
- } else {
- ap_rputs("<dd><tt>", r);
- mod_info_html_cmd_string(r, tmptree->directive, 0);
- ap_rprintf(r, " <i>%s</i></tt></dd>\n",
- ap_escape_html(r->pool,tmptree->args));
- }
- }
- ++cmd;
- }
- if (tmptree->first_child != NULL) {
- tmptree = tmptree->first_child;
- nest++;
- } else if (tmptree->next != NULL) {
- tmptree = tmptree->next;
- } else {
- if (block_start) {
- apr_snprintf(htmlstring, sizeof(htmlstring), "%s %s",
- tmptree->parent->directive,
- tmptree->parent->args);
- ap_rputs("<dd><tt>", r);
- mod_info_html_cmd_string(r, htmlstring, 1);
- ap_rputs("</tt></dd>\n", r);
- block_start--;
- }
- if (tmptree->parent) {
- tmptree = tmptree->parent->next;
+ mod_info_indent(r, nest, dir->filename, dir->line_num);
+ ap_rprintf(r, "%s <i>%s</i></tt></dd>\n",
+ ap_escape_html(r->pool, dir->directive),
+ ap_escape_html(r->pool, dir->args));
+}
+
+static void mod_info_show_open(request_rec * r, const ap_directive_t * dir,
+ int nest)
+{
+ mod_info_indent(r, nest, dir->filename, dir->line_num);
+ ap_rprintf(r, "%s %s</tt></dd>\n",
+ ap_escape_html(r->pool, dir->directive),
+ ap_escape_html(r->pool, dir->args));
+}
+
+static void mod_info_show_close(request_rec * r, const ap_directive_t * dir,
+ int nest)
+{
+ const char *dirname = dir->directive;
+ mod_info_indent(r, nest, dir->filename, 0);
+ if (*dirname == '<') {
+ ap_rprintf(r, "</%s></tt></dd>",
+ ap_escape_html(r->pool, dirname + 1));
}
else {
- tmptree = NULL;
+ ap_rprintf(r, "/%s</tt></dd>",
+ ap_escape_html(r->pool, dirname));
}
- nest--;
+}
+
+static int mod_info_has_cmd(const command_rec * cmds, ap_directive_t * dir)
+{
+ const command_rec * cmd;
+ if (cmds == NULL) return 1;
+ for (cmd = cmds; cmd->name; ++cmd) {
+ if (strcasecmp(cmd->name, dir->directive) == 0) return 1;
}
+ return 0;
+}
+
+static void mod_info_show_parents(request_rec * r, ap_directive_t * node,
+ int from, int to) {
+ if (from < to) mod_info_show_parents(r, node->parent, from, to - 1);
+ mod_info_show_open(r, node, to);
+}
+static int mod_info_module_cmds(request_rec * r, const command_rec * cmds,
+ ap_directive_t * node, int from, int level)
+{
+ int shown = from;
+ ap_directive_t * dir;
+ if (level == 0) ap_set_module_config(r->request_config, &info_module, NULL);
+ for (dir = node; dir; dir = dir->next) {
+ if (dir->first_child != NULL) {
+ if (level < mod_info_module_cmds(r, cmds, dir->first_child,
+ shown, level + 1)) {
+ shown = level;
+ mod_info_show_close(r, dir, level);
+ }
+ } else if (mod_info_has_cmd(cmds, dir)) {
+ if (shown < level) {
+ mod_info_show_parents(r, dir->parent, shown, level - 1);
+ shown = level;
+ }
+ mod_info_show_cmd(r, dir, level);
+ }
}
+ return shown;
}
typedef struct { /*XXX: should get something from apr_hooks.h instead */
@@ -368,6 +364,12 @@
"<tt>%s</tt></dt>\n", ap_conftree->filename);
ap_rputs("</dl><hr />", r);
}
+ if (r->args && 0 == strcasecmp(r->args, "config")) {
+ ap_rputs("<dl><dt><strong>Configuration:</strong>\n", r);
+ mod_info_module_cmds(r, NULL, ap_conftree, 0, 0);
+ ap_rputs("</dl><hr />", r);
+ }
+ else {
for (modp = ap_top_module; modp; modp = modp->next) {
if (!r->args || !strcasecmp(modp->name, r->args)) {
ap_rprintf(r, "<dl><dt><a name=\"%s\"><strong>Module Name:</strong> "
@@ -444,9 +446,9 @@
ap_rputs("<dt><strong>Module Directives:</strong></dt>", r);
while (cmd) {
if (cmd->name) {
- ap_rputs("<dd><tt>", r);
- mod_info_html_cmd_string(r, cmd->name, 0);
- ap_rputs(" - <i>", r);
+ ap_rprintf(r, "<dd><tt>%s%s - <i>",
+ ap_escape_html(r->pool, cmd->name),
+ cmd->name[0] == '<' ? ">" : "");
if (cmd->errmsg) {
ap_rputs(cmd->errmsg, r);
}
@@ -458,7 +460,7 @@
cmd++;
}
ap_rputs("<dt><strong>Current Configuration:</strong></dt>\n", r);
- mod_info_module_cmds(r, modp->cmds, ap_conftree);
+ mod_info_module_cmds(r, modp->cmds, ap_conftree, 0, 0);
}
else {
ap_rputs("<dt><strong>Module Directives:</strong> <tt>none</tt></dt>", r);
@@ -478,6 +480,7 @@
}
if (!modp && r->args && strcasecmp(r->args, "server")) {
ap_rputs("<p><b>No such module</b></p>\n", r);
+ }
}
}
else {
Re: cvs commit: httpd-2.0/modules/generators mod_info.c
Posted by "Roy T. Fielding" <fi...@gbiv.com>.
On Sep 3, 2004, at 6:06 PM, Paul Querna wrote:
> On Fri, 2004-09-03 at 16:06 -0700, Roy T. Fielding wrote:
>> -0.9. This change needs review. The coding style should stick to
>> the httpd guidelines.
>
> Yes, but I did not commit a correct style patch, because that would of
> been *impossible* to review. The style of mod_info does not follow the
> guidelines, and Rici's original patch was much larger (including style
> fixes). My plan was to make his functional changes first, and come
> back
> later this weekend for a style cleanup of mod_info.
okay, that's a reasonable plan -- feel free to mention that in the
cvs log next time.
>> Also, we don't add comments to a credit log inside the file
>> (instead of just CHANGES), and whether or not God intended
>> recursion is not relevant (though that part of the changes
>> does look good).
>
> What is the Policy on this? Should we remove the old ones from
> existing
> files? Sort of like how @author tags have been removed from other ASF
> projects...
We keep around things like "Originally written by ..." for entire
files, but nothing else. Feel free to send a patch to the list if
you are unsure about something -- stuff like that takes time to
figure out the unwritten policy.
>> It is unfortunate that the diff gets confused about the new functions
>> and treats them as changes -- it makes review a pain in the butt.
>
> Like I said, this is the minimal patch to get the functional
> differences
> for mod_info. mod_info already did not fit the style guide, and making
> Functional and Style changes in the same commit is even harder to
> review.
Well, in this case the cvs diff screwed the review anyway, but I
was actually talking about the style of the new code. Feel free
to fix that later, but I figured I should mention it just in case
you hadn't seen the guidelines yet.
Style is important because it makes it easier to review changes
like that one. I actually prefer to commit the style fix first
and then the change, since things like the usage of sprintf are
easier to see under our normal style. This part just makes me
uncomfortable, like walking down a dark alley at night:
+ char buf[32];
+ if (thisfn == NULL) thisfn = "*UNKNOWN*";
+ if (prevfn == NULL || 0 != strcmp(prevfn, thisfn)) {
+ thisfn = ap_escape_html(r->pool, thisfn);
+ ap_rprintf(r, "<dd><tt><strong>In file:
%s</strong></tt></dd>\n", thisfn);
+ ap_set_module_config(r->request_config, &info_module,
thisfn);
}
+ ap_rputs("<dd><tt>", r);
+ if (linenum > 0) sprintf(buf, "%d", linenum);
+ else buf[0] = '\0';
+ for (i = strlen(buf); i < 4; ++i) ap_rputs(" ", r);
+ ap_rputs(buf, r);
+ ap_rputs(": ", r);
+ for (i = 1; i <= nest; ++i) ap_rputs(" ", r);
....Roy
Re: cvs commit: httpd-2.0/modules/generators mod_info.c
Posted by Paul Querna <ch...@force-elite.com>.
On Fri, 2004-09-03 at 16:06 -0700, Roy T. Fielding wrote:
> -0.9. This change needs review. The coding style should stick to
> the httpd guidelines.
Yes, but I did not commit a correct style patch, because that would of
been *impossible* to review. The style of mod_info does not follow the
guidelines, and Rici's original patch was much larger (including style
fixes). My plan was to make his functional changes first, and come back
later this weekend for a style cleanup of mod_info.
> Also, we don't add comments to a credit log inside the file
> (instead of just CHANGES), and whether or not God intended
> recursion is not relevant (though that part of the changes
> does look good).
What is the Policy on this? Should we remove the old ones from existing
files? Sort of like how @author tags have been removed from other ASF
projects...
> It is unfortunate that the diff gets confused about the new functions
> and treats them as changes -- it makes review a pain in the butt.
Like I said, this is the minimal patch to get the functional differences
for mod_info. mod_info already did not fit the style guide, and making
Functional and Style changes in the same commit is even harder to
review.
> ....Roy
>
>
> On Sep 2, 2004, at 7:31 PM, pquerna@apache.org wrote:
>
> > pquerna 2004/09/02 19:31:06
> >
> > Modified: . CHANGES
> > modules/generators mod_info.c
> > Log:
> > Rewrote config tree walk using recursion the way God intended.
> > Added ?config option. Added printout of config filename and line
> > numbers.
> >
> > PR: 30919
> > Submitted by: Rici Lake <rici ricilake.net>
> >
> > Revision Changes Path
> > 1.1584 +4 -0 httpd-2.0/CHANGES
> >
> > Index: CHANGES
> > ===================================================================
> > RCS file: /home/cvs/httpd-2.0/CHANGES,v
> > retrieving revision 1.1583
> > retrieving revision 1.1584
> > diff -u -r1.1583 -r1.1584
> > --- CHANGES 2 Sep 2004 19:49:19 -0000 1.1583
> > +++ CHANGES 3 Sep 2004 02:31:05 -0000 1.1584
> > @@ -2,6 +2,10 @@
> >
> > [Remove entries to the current 2.0 section below, when backported]
> >
> > + *) mod_info: Rewrote config tree walk using a recursive function.
> > + Added ?config option. Added printout of config filename and
> > line numbers.
> > + [Rici Lake <rici ricilake.net>, Paul Querna]
> > +
> > *) mod_proxy: Fix type error that prevents proxy-sendchunks from
> > working.
> > [Justin Erenkrantz]
> >
> >
> >
> >
> > 1.57 +98 -95 httpd-2.0/modules/generators/mod_info.c
> >
> > Index: mod_info.c
> > ===================================================================
> > RCS file: /home/cvs/httpd-2.0/modules/generators/mod_info.c,v
> > retrieving revision 1.56
> > retrieving revision 1.57
> > diff -u -r1.56 -r1.57
> > --- mod_info.c 9 Feb 2004 20:29:19 -0000 1.56
> > +++ mod_info.c 3 Sep 2004 02:31:06 -0000 1.57
> > @@ -25,6 +25,7 @@
> > * GET /server-info?server - Returns server configuration only
> > * GET /server-info?module_name - Returns configuration for a
> > single module
> > * GET /server-info?list - Returns quick list of included modules
> > + * GET /server-info?config - Returns full configuration
> > *
> > * Rasmus Lerdorf <ra...@vex.net>, May 1996
> > *
> > @@ -39,6 +40,12 @@
> > * 8.11.00 Port to Apache 2.0. Read configuation from the
> > configuration
> > * tree rather than reparse the entire configuation file.
> > *
> > + * Rici Lake <ri...@ricilake.net>
> > + *
> > + * 2004-08-28 Rewrote config tree walk using recursion the way God
> > intended.
> > + * Added ?config option. Added printout of config filename and line
> > numbers.
> > + * Fixed indentation.
> > + *
> > */
> >
> > #define CORE_PRIVATE
> > @@ -86,108 +93,97 @@
> > return new;
> > }
> >
> > -static void mod_info_html_cmd_string(request_rec *r, const char
> > *string,
> > - int close)
> > +static void mod_info_indent(request_rec * r, int nest, const char*
> > thisfn, int linenum)
> > {
> > - const char *s;
> > -
> > - s = string;
> > - /* keep space for \0 byte */
> > - while (*s) {
> > - if (*s == '<') {
> > - if (close) {
> > - ap_rputs("</", r);
> > - } else {
> > - ap_rputs("<", r);
> > - }
> > - }
> > - else if (*s == '>') {
> > - ap_rputs(">", r);
> > - }
> > - else if (*s == '&') {
> > - ap_rputs("&", r);
> > - }
> > - else if (*s == ' ') {
> > - if (close) {
> > - ap_rputs(">", r);
> > - break;
> > - } else {
> > - ap_rputc(*s, r);
> > - }
> > - } else {
> > - ap_rputc(*s, r);
> > - }
> > - s++;
> > + int i;
> > + const char *prevfn = ap_get_module_config(r->request_config,
> > &info_module);
> > + char buf[32];
> > + if (thisfn == NULL) thisfn = "*UNKNOWN*";
> > + if (prevfn == NULL || 0 != strcmp(prevfn, thisfn)) {
> > + thisfn = ap_escape_html(r->pool, thisfn);
> > + ap_rprintf(r, "<dd><tt><strong>In file:
> > %s</strong></tt></dd>\n", thisfn);
> > + ap_set_module_config(r->request_config, &info_module,
> > thisfn);
> > }
> > + ap_rputs("<dd><tt>", r);
> > + if (linenum > 0) sprintf(buf, "%d", linenum);
> > + else buf[0] = '\0';
> > + for (i = strlen(buf); i < 4; ++i) ap_rputs(" ", r);
> > + ap_rputs(buf, r);
> > + ap_rputs(": ", r);
> > + for (i = 1; i <= nest; ++i) ap_rputs(" ", r);
> > }
> >
> > -static void mod_info_module_cmds(request_rec * r, const command_rec
> > * cmds,
> > - ap_directive_t * conftree)
> > +static void mod_info_show_cmd(request_rec * r, const ap_directive_t
> > * dir,
> > + int nest)
> > {
> > - const command_rec *cmd;
> > - ap_directive_t *tmptree = conftree;
> > - char htmlstring[MAX_STRING_LEN];
> > - int block_start = 0;
> > - int nest = 0;
> > -
> > - while (tmptree != NULL) {
> > - cmd = cmds;
> > - while (cmd->name) {
> > - if ((cmd->name[0] != '<') &&
> > - (strcasecmp(cmd->name, tmptree->directive) == 0)) {
> > - if (nest > block_start) {
> > - block_start++;
> > - apr_snprintf(htmlstring, sizeof(htmlstring), "%s %s",
> > - tmptree->parent->directive,
> > - tmptree->parent->args);
> > - ap_rputs("<dd><tt>", r);
> > - mod_info_html_cmd_string(r, htmlstring, 0);
> > - ap_rputs("</tt></dd>\n", r);
> > - }
> > - if (nest == 2) {
> > - ap_rprintf(r, "<dd><tt> %s "
> > - "<i>%s</i></tt></dd>\n",
> > -
> > ap_escape_html(r->pool,tmptree->directive),
> > -
> > ap_escape_html(r->pool,tmptree->args));
> > - } else if (nest == 1) {
> > - ap_rprintf(r,
> > - "<dd><tt> %s <i>%s</i></tt></dd>\n",
> > -
> > ap_escape_html(r->pool,tmptree->directive),
> > -
> > ap_escape_html(r->pool,tmptree->args));
> > - } else {
> > - ap_rputs("<dd><tt>", r);
> > - mod_info_html_cmd_string(r, tmptree->directive,
> > 0);
> > - ap_rprintf(r, " <i>%s</i></tt></dd>\n",
> > -
> > ap_escape_html(r->pool,tmptree->args));
> > - }
> > - }
> > - ++cmd;
> > - }
> > - if (tmptree->first_child != NULL) {
> > - tmptree = tmptree->first_child;
> > - nest++;
> > - } else if (tmptree->next != NULL) {
> > - tmptree = tmptree->next;
> > - } else {
> > - if (block_start) {
> > - apr_snprintf(htmlstring, sizeof(htmlstring), "%s %s",
> > - tmptree->parent->directive,
> > - tmptree->parent->args);
> > - ap_rputs("<dd><tt>", r);
> > - mod_info_html_cmd_string(r, htmlstring, 1);
> > - ap_rputs("</tt></dd>\n", r);
> > - block_start--;
> > - }
> > - if (tmptree->parent) {
> > - tmptree = tmptree->parent->next;
> > + mod_info_indent(r, nest, dir->filename, dir->line_num);
> > + ap_rprintf(r, "%s <i>%s</i></tt></dd>\n",
> > + ap_escape_html(r->pool, dir->directive),
> > + ap_escape_html(r->pool, dir->args));
> > +}
> > +
> > +static void mod_info_show_open(request_rec * r, const
> > ap_directive_t * dir,
> > + int nest)
> > +{
> > + mod_info_indent(r, nest, dir->filename, dir->line_num);
> > + ap_rprintf(r, "%s %s</tt></dd>\n",
> > + ap_escape_html(r->pool, dir->directive),
> > + ap_escape_html(r->pool, dir->args));
> > +}
> > +
> > +static void mod_info_show_close(request_rec * r, const
> > ap_directive_t * dir,
> > + int nest)
> > +{
> > + const char *dirname = dir->directive;
> > + mod_info_indent(r, nest, dir->filename, 0);
> > + if (*dirname == '<') {
> > + ap_rprintf(r, "</%s></tt></dd>",
> > + ap_escape_html(r->pool, dirname + 1));
> > }
> > else {
> > - tmptree = NULL;
> > + ap_rprintf(r, "/%s</tt></dd>",
> > + ap_escape_html(r->pool, dirname));
> > }
> > - nest--;
> > +}
> > +
> > +static int mod_info_has_cmd(const command_rec * cmds,
> > ap_directive_t * dir)
> > +{
> > + const command_rec * cmd;
> > + if (cmds == NULL) return 1;
> > + for (cmd = cmds; cmd->name; ++cmd) {
> > + if (strcasecmp(cmd->name, dir->directive) == 0) return 1;
> > }
> > + return 0;
> > +}
> > +
> > +static void mod_info_show_parents(request_rec * r, ap_directive_t *
> > node,
> > + int from, int to) {
> > + if (from < to) mod_info_show_parents(r, node->parent, from, to
> > - 1);
> > + mod_info_show_open(r, node, to);
> > +}
> >
> > +static int mod_info_module_cmds(request_rec * r, const command_rec
> > * cmds,
> > + ap_directive_t * node, int from,
> > int level)
> > +{
> > + int shown = from;
> > + ap_directive_t * dir;
> > + if (level == 0) ap_set_module_config(r->request_config,
> > &info_module, NULL);
> > + for (dir = node; dir; dir = dir->next) {
> > + if (dir->first_child != NULL) {
> > + if (level < mod_info_module_cmds(r, cmds,
> > dir->first_child,
> > + shown, level + 1))
> > {
> > + shown = level;
> > + mod_info_show_close(r, dir, level);
> > + }
> > + } else if (mod_info_has_cmd(cmds, dir)) {
> > + if (shown < level) {
> > + mod_info_show_parents(r, dir->parent, shown, level
> > - 1);
> > + shown = level;
> > + }
> > + mod_info_show_cmd(r, dir, level);
> > + }
> > }
> > + return shown;
> > }
> >
> > typedef struct { /*XXX: should get something from apr_hooks.h
> > instead */
> > @@ -368,6 +364,12 @@
> > "<tt>%s</tt></dt>\n", ap_conftree->filename);
> > ap_rputs("</dl><hr />", r);
> > }
> > + if (r->args && 0 == strcasecmp(r->args, "config")) {
> > + ap_rputs("<dl><dt><strong>Configuration:</strong>\n",
> > r);
> > + mod_info_module_cmds(r, NULL, ap_conftree, 0, 0);
> > + ap_rputs("</dl><hr />", r);
> > + }
> > + else {
> > for (modp = ap_top_module; modp; modp = modp->next) {
> > if (!r->args || !strcasecmp(modp->name, r->args)) {
> > ap_rprintf(r, "<dl><dt><a
> > name=\"%s\"><strong>Module Name:</strong> "
> > @@ -444,9 +446,9 @@
> > ap_rputs("<dt><strong>Module
> > Directives:</strong></dt>", r);
> > while (cmd) {
> > if (cmd->name) {
> > - ap_rputs("<dd><tt>", r);
> > - mod_info_html_cmd_string(r, cmd->name,
> > 0);
> > - ap_rputs(" - <i>", r);
> > + ap_rprintf(r, "<dd><tt>%s%s - <i>",
> > + ap_escape_html(r->pool, cmd->name),
> > + cmd->name[0] == '<' ? ">" : "");
> > if (cmd->errmsg) {
> > ap_rputs(cmd->errmsg, r);
> > }
> > @@ -458,7 +460,7 @@
> > cmd++;
> > }
> > ap_rputs("<dt><strong>Current
> > Configuration:</strong></dt>\n", r);
> > - mod_info_module_cmds(r, modp->cmds,
> > ap_conftree);
> > + mod_info_module_cmds(r, modp->cmds,
> > ap_conftree, 0, 0);
> > }
> > else {
> > ap_rputs("<dt><strong>Module
> > Directives:</strong> <tt>none</tt></dt>", r);
> > @@ -478,6 +480,7 @@
> > }
> > if (!modp && r->args && strcasecmp(r->args, "server")) {
> > ap_rputs("<p><b>No such module</b></p>\n", r);
> > + }
> > }
> > }
> > else {
> >
> >
> >
> >
> >
>
Re: cvs commit: httpd-2.0/modules/generators mod_info.c
Posted by "Roy T. Fielding" <fi...@gbiv.com>.
-0.9. This change needs review. The coding style should stick to
the httpd guidelines. Use of sprintf into a small fixed size
buffer is unwise (even when we know it fits) .. use
apr_snprintf.
Also, we don't add comments to a credit log inside the file
(instead of just CHANGES), and whether or not God intended
recursion is not relevant (though that part of the changes
does look good).
It is unfortunate that the diff gets confused about the new functions
and treats them as changes -- it makes review a pain in the butt.
....Roy
On Sep 2, 2004, at 7:31 PM, pquerna@apache.org wrote:
> pquerna 2004/09/02 19:31:06
>
> Modified: . CHANGES
> modules/generators mod_info.c
> Log:
> Rewrote config tree walk using recursion the way God intended.
> Added ?config option. Added printout of config filename and line
> numbers.
>
> PR: 30919
> Submitted by: Rici Lake <rici ricilake.net>
>
> Revision Changes Path
> 1.1584 +4 -0 httpd-2.0/CHANGES
>
> Index: CHANGES
> ===================================================================
> RCS file: /home/cvs/httpd-2.0/CHANGES,v
> retrieving revision 1.1583
> retrieving revision 1.1584
> diff -u -r1.1583 -r1.1584
> --- CHANGES 2 Sep 2004 19:49:19 -0000 1.1583
> +++ CHANGES 3 Sep 2004 02:31:05 -0000 1.1584
> @@ -2,6 +2,10 @@
>
> [Remove entries to the current 2.0 section below, when backported]
>
> + *) mod_info: Rewrote config tree walk using a recursive function.
> + Added ?config option. Added printout of config filename and
> line numbers.
> + [Rici Lake <rici ricilake.net>, Paul Querna]
> +
> *) mod_proxy: Fix type error that prevents proxy-sendchunks from
> working.
> [Justin Erenkrantz]
>
>
>
>
> 1.57 +98 -95 httpd-2.0/modules/generators/mod_info.c
>
> Index: mod_info.c
> ===================================================================
> RCS file: /home/cvs/httpd-2.0/modules/generators/mod_info.c,v
> retrieving revision 1.56
> retrieving revision 1.57
> diff -u -r1.56 -r1.57
> --- mod_info.c 9 Feb 2004 20:29:19 -0000 1.56
> +++ mod_info.c 3 Sep 2004 02:31:06 -0000 1.57
> @@ -25,6 +25,7 @@
> * GET /server-info?server - Returns server configuration only
> * GET /server-info?module_name - Returns configuration for a
> single module
> * GET /server-info?list - Returns quick list of included modules
> + * GET /server-info?config - Returns full configuration
> *
> * Rasmus Lerdorf <ra...@vex.net>, May 1996
> *
> @@ -39,6 +40,12 @@
> * 8.11.00 Port to Apache 2.0. Read configuation from the
> configuration
> * tree rather than reparse the entire configuation file.
> *
> + * Rici Lake <ri...@ricilake.net>
> + *
> + * 2004-08-28 Rewrote config tree walk using recursion the way God
> intended.
> + * Added ?config option. Added printout of config filename and line
> numbers.
> + * Fixed indentation.
> + *
> */
>
> #define CORE_PRIVATE
> @@ -86,108 +93,97 @@
> return new;
> }
>
> -static void mod_info_html_cmd_string(request_rec *r, const char
> *string,
> - int close)
> +static void mod_info_indent(request_rec * r, int nest, const char*
> thisfn, int linenum)
> {
> - const char *s;
> -
> - s = string;
> - /* keep space for \0 byte */
> - while (*s) {
> - if (*s == '<') {
> - if (close) {
> - ap_rputs("</", r);
> - } else {
> - ap_rputs("<", r);
> - }
> - }
> - else if (*s == '>') {
> - ap_rputs(">", r);
> - }
> - else if (*s == '&') {
> - ap_rputs("&", r);
> - }
> - else if (*s == ' ') {
> - if (close) {
> - ap_rputs(">", r);
> - break;
> - } else {
> - ap_rputc(*s, r);
> - }
> - } else {
> - ap_rputc(*s, r);
> - }
> - s++;
> + int i;
> + const char *prevfn = ap_get_module_config(r->request_config,
> &info_module);
> + char buf[32];
> + if (thisfn == NULL) thisfn = "*UNKNOWN*";
> + if (prevfn == NULL || 0 != strcmp(prevfn, thisfn)) {
> + thisfn = ap_escape_html(r->pool, thisfn);
> + ap_rprintf(r, "<dd><tt><strong>In file:
> %s</strong></tt></dd>\n", thisfn);
> + ap_set_module_config(r->request_config, &info_module,
> thisfn);
> }
> + ap_rputs("<dd><tt>", r);
> + if (linenum > 0) sprintf(buf, "%d", linenum);
> + else buf[0] = '\0';
> + for (i = strlen(buf); i < 4; ++i) ap_rputs(" ", r);
> + ap_rputs(buf, r);
> + ap_rputs(": ", r);
> + for (i = 1; i <= nest; ++i) ap_rputs(" ", r);
> }
>
> -static void mod_info_module_cmds(request_rec * r, const command_rec
> * cmds,
> - ap_directive_t * conftree)
> +static void mod_info_show_cmd(request_rec * r, const ap_directive_t
> * dir,
> + int nest)
> {
> - const command_rec *cmd;
> - ap_directive_t *tmptree = conftree;
> - char htmlstring[MAX_STRING_LEN];
> - int block_start = 0;
> - int nest = 0;
> -
> - while (tmptree != NULL) {
> - cmd = cmds;
> - while (cmd->name) {
> - if ((cmd->name[0] != '<') &&
> - (strcasecmp(cmd->name, tmptree->directive) == 0)) {
> - if (nest > block_start) {
> - block_start++;
> - apr_snprintf(htmlstring, sizeof(htmlstring), "%s %s",
> - tmptree->parent->directive,
> - tmptree->parent->args);
> - ap_rputs("<dd><tt>", r);
> - mod_info_html_cmd_string(r, htmlstring, 0);
> - ap_rputs("</tt></dd>\n", r);
> - }
> - if (nest == 2) {
> - ap_rprintf(r, "<dd><tt> %s "
> - "<i>%s</i></tt></dd>\n",
> -
> ap_escape_html(r->pool,tmptree->directive),
> -
> ap_escape_html(r->pool,tmptree->args));
> - } else if (nest == 1) {
> - ap_rprintf(r,
> - "<dd><tt> %s <i>%s</i></tt></dd>\n",
> -
> ap_escape_html(r->pool,tmptree->directive),
> -
> ap_escape_html(r->pool,tmptree->args));
> - } else {
> - ap_rputs("<dd><tt>", r);
> - mod_info_html_cmd_string(r, tmptree->directive,
> 0);
> - ap_rprintf(r, " <i>%s</i></tt></dd>\n",
> -
> ap_escape_html(r->pool,tmptree->args));
> - }
> - }
> - ++cmd;
> - }
> - if (tmptree->first_child != NULL) {
> - tmptree = tmptree->first_child;
> - nest++;
> - } else if (tmptree->next != NULL) {
> - tmptree = tmptree->next;
> - } else {
> - if (block_start) {
> - apr_snprintf(htmlstring, sizeof(htmlstring), "%s %s",
> - tmptree->parent->directive,
> - tmptree->parent->args);
> - ap_rputs("<dd><tt>", r);
> - mod_info_html_cmd_string(r, htmlstring, 1);
> - ap_rputs("</tt></dd>\n", r);
> - block_start--;
> - }
> - if (tmptree->parent) {
> - tmptree = tmptree->parent->next;
> + mod_info_indent(r, nest, dir->filename, dir->line_num);
> + ap_rprintf(r, "%s <i>%s</i></tt></dd>\n",
> + ap_escape_html(r->pool, dir->directive),
> + ap_escape_html(r->pool, dir->args));
> +}
> +
> +static void mod_info_show_open(request_rec * r, const
> ap_directive_t * dir,
> + int nest)
> +{
> + mod_info_indent(r, nest, dir->filename, dir->line_num);
> + ap_rprintf(r, "%s %s</tt></dd>\n",
> + ap_escape_html(r->pool, dir->directive),
> + ap_escape_html(r->pool, dir->args));
> +}
> +
> +static void mod_info_show_close(request_rec * r, const
> ap_directive_t * dir,
> + int nest)
> +{
> + const char *dirname = dir->directive;
> + mod_info_indent(r, nest, dir->filename, 0);
> + if (*dirname == '<') {
> + ap_rprintf(r, "</%s></tt></dd>",
> + ap_escape_html(r->pool, dirname + 1));
> }
> else {
> - tmptree = NULL;
> + ap_rprintf(r, "/%s</tt></dd>",
> + ap_escape_html(r->pool, dirname));
> }
> - nest--;
> +}
> +
> +static int mod_info_has_cmd(const command_rec * cmds,
> ap_directive_t * dir)
> +{
> + const command_rec * cmd;
> + if (cmds == NULL) return 1;
> + for (cmd = cmds; cmd->name; ++cmd) {
> + if (strcasecmp(cmd->name, dir->directive) == 0) return 1;
> }
> + return 0;
> +}
> +
> +static void mod_info_show_parents(request_rec * r, ap_directive_t *
> node,
> + int from, int to) {
> + if (from < to) mod_info_show_parents(r, node->parent, from, to
> - 1);
> + mod_info_show_open(r, node, to);
> +}
>
> +static int mod_info_module_cmds(request_rec * r, const command_rec
> * cmds,
> + ap_directive_t * node, int from,
> int level)
> +{
> + int shown = from;
> + ap_directive_t * dir;
> + if (level == 0) ap_set_module_config(r->request_config,
> &info_module, NULL);
> + for (dir = node; dir; dir = dir->next) {
> + if (dir->first_child != NULL) {
> + if (level < mod_info_module_cmds(r, cmds,
> dir->first_child,
> + shown, level + 1))
> {
> + shown = level;
> + mod_info_show_close(r, dir, level);
> + }
> + } else if (mod_info_has_cmd(cmds, dir)) {
> + if (shown < level) {
> + mod_info_show_parents(r, dir->parent, shown, level
> - 1);
> + shown = level;
> + }
> + mod_info_show_cmd(r, dir, level);
> + }
> }
> + return shown;
> }
>
> typedef struct { /*XXX: should get something from apr_hooks.h
> instead */
> @@ -368,6 +364,12 @@
> "<tt>%s</tt></dt>\n", ap_conftree->filename);
> ap_rputs("</dl><hr />", r);
> }
> + if (r->args && 0 == strcasecmp(r->args, "config")) {
> + ap_rputs("<dl><dt><strong>Configuration:</strong>\n",
> r);
> + mod_info_module_cmds(r, NULL, ap_conftree, 0, 0);
> + ap_rputs("</dl><hr />", r);
> + }
> + else {
> for (modp = ap_top_module; modp; modp = modp->next) {
> if (!r->args || !strcasecmp(modp->name, r->args)) {
> ap_rprintf(r, "<dl><dt><a
> name=\"%s\"><strong>Module Name:</strong> "
> @@ -444,9 +446,9 @@
> ap_rputs("<dt><strong>Module
> Directives:</strong></dt>", r);
> while (cmd) {
> if (cmd->name) {
> - ap_rputs("<dd><tt>", r);
> - mod_info_html_cmd_string(r, cmd->name,
> 0);
> - ap_rputs(" - <i>", r);
> + ap_rprintf(r, "<dd><tt>%s%s - <i>",
> + ap_escape_html(r->pool, cmd->name),
> + cmd->name[0] == '<' ? ">" : "");
> if (cmd->errmsg) {
> ap_rputs(cmd->errmsg, r);
> }
> @@ -458,7 +460,7 @@
> cmd++;
> }
> ap_rputs("<dt><strong>Current
> Configuration:</strong></dt>\n", r);
> - mod_info_module_cmds(r, modp->cmds,
> ap_conftree);
> + mod_info_module_cmds(r, modp->cmds,
> ap_conftree, 0, 0);
> }
> else {
> ap_rputs("<dt><strong>Module
> Directives:</strong> <tt>none</tt></dt>", r);
> @@ -478,6 +480,7 @@
> }
> if (!modp && r->args && strcasecmp(r->args, "server")) {
> ap_rputs("<p><b>No such module</b></p>\n", r);
> + }
> }
> }
> else {
>
>
>
>
>