On 29-Nov-04, at 11:02 AM, William A. Rowe, Jr. wrote:
Yes, but our API dictates that they are looking for ...> (in the case of this module) - which is a bad thing IMHO (and in Greg's and Ryan's as well)...
Sorry, I don't understand that. The search for > is in the first line
of the directive; arg is simply the arg delivered by the config apparatus.
The point of the following "offending" code is to ensure that the
directive was:
<Comment This is a comment>
and not:
<Comment This is a comment
I'm actually neutral on whether it is worth doing this check, but all the
other section directives do it, and while it seems a bit puritanical, it
does enforce the vaguely HTML-like section syntax.
This is the offending code;
endp = ap_strrchr_c(arg, '>'); if (endp == NULL) { return unclosed_directive(cmd); }
So how is that different from parsing any other standard directive? It is no different than mod_access, for example, checking that the first argument to Allow is the word "from".
and this is the good code;
*(ap_directive_t **)dummy = NULL; return ap_soak_end_container(cmd, "<Comment");
It will be noted that this module does not in any way refer to the fp hidden away in the structs. All it does is use the public API. If the internal functioning of that API changes, the module will continue to work.
The problem is, I doubt this module inserts its contents into the config tree.
That is true, it doesn't. The *(ap_directive_t **)dummy = NULL; statement tells the config apparatus that there are no directives to insert. This is the part of the module that I personally object to; that behaviour is non-standard, undocumented, and a kludge. However, "everybody does it".
So if we spew the contents (using mod_info, or any other tool based on our config tree API) we lose the contents.
I would be happy to fix this :) It is worth noting that EXEC_ON_READ directives are all removed from the config_tree, so that mod_info cannot show useful things like LoadModule directives.
