On Friday 10 August 2007 20:42, Pierre Métras wrote:
> Here is the third revision of my patch to add support for user defined
> error pages in httpd. I've integrated most of the comments from Denis
> Vlasenko, but one. During development, I was surprised to get a larger
> executable. The parse_conf() function got the biggest code size increase.
>
> After a few tests, it seems the culprit is the suggested bb_strtoi() call.
> The following line adds 97 bytes (on PIV with gcc):
> int status = bb_strtoi(p0 + 1, &p0, 10)
> function old new delta
> parse_conf 1201 1448 +247
>
> compared with originally:
> c = strchr(++p0, ':');
> *c = 0;
> int status = xatoi(p0);
> p0 = ++c;
> function old new delta
> parse_conf 1201 1337 +136
>
> I couldn't explain why such a difference...
>
> This patch includes:
> - Support for definition of error pages in httpd.conf (it was the main
> goal!). This is enabled by a configuration option, default no.
> - Reports of simple errors in httpd.conf when they are detected. It does
> not check that the error page exists. Reports of memory exhausted.
Please rediff against current svn and resend. Unfortunately it collides with
your other patch (sendfile support) which I just applied with changes.
> - Support for comments and empty lines in httpd.conf. End of line comments
> where already supported and not documented.
Can you submit it as separate patch? (Two patches in one mail is ok with me).
Review:
-static const HttpEnumString httpResponseNames[] = {
- { HTTP_OK, "OK", NULL },
- { HTTP_MOVED_TEMPORARILY, "Found", "Directories must end with a slash."
},
+static SKIP_FEATURE_HTTPD_ERROR_PAGES(const) HttpEnumString
httpResponseNames[] = {
+ { HTTP_OK, "OK", NULL USE_FEATURE_HTTPD_ERROR_PAGES(, NULL) },
+ { HTTP_MOVED_TEMPORARILY, "Found", "Directories must end with a slash."
USE_FEATURE_HTTPD_ERROR_PAGES(, NULL) },
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
IIRC you can simply omit NULL/0 trailing members in struct initializer.
Will look much less ugly here.
@@ -488,22 +508,30 @@
#endif
/* This could stand some work */
while ((p0 = fgets(buf, sizeof(buf), f)) != NULL) {
+ /* Comments: '#' as first character of the line */
+ if (*p0 == '#' || *p0 == '\n')
+ continue;
+
c = NULL;
for (p = p0; *p0 != 0 && *p0 != '#'; p0++) {
if (!isspace(*p0)) {
*p++ = *p0;
if (*p0 == ':' && c == NULL)
- c = p;
+ c = p;
}
}
*p = 0;
- /* test for empty or strange line */
- if (c == NULL || *c == 0)
+ /* test for empty or strange line
+ ':' is present on all syntaxes
+ and c points at the char after it */
+ if (c == NULL || *c == 0) {
+ bb_error_msg("config error '%s' in '%s'", buf, cf);
continue;
+ }
p0 = buf;
if (*p0 == 'd')
- *p0 = 'D';
+ *p0 = 'D';
if (*c == '*') {
if (*p0 == 'D') {
/* memorize deny all */
@@ -515,18 +543,6 @@
if (*p0 == 'a')
*p0 = 'A';
- else if (*p0 != 'D' && *p0 != 'A'
-#if ENABLE_FEATURE_HTTPD_BASIC_AUTH
- && *p0 != '/'
-#endif
-#if ENABLE_FEATURE_HTTPD_CONFIG_WITH_MIME_TYPES
- && *p0 != '.'
-#endif
-#if ENABLE_FEATURE_HTTPD_CONFIG_WITH_SCRIPT_INTERPR
- && *p0 != '*'
-#endif
- )
- continue;
if (*p0 == 'A' || *p0 == 'D') {
/* storing current config IP line */
pip = xzalloc(sizeof(Htaccess_IP));
Above two hunks are not immediately obvious. What do they do?
If it's unrelated to error pages support, submit separately.
--
vda
_______________________________________________
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox
