On Thursday 12 July 2007 01:39, Pierre Métras wrote:
> Hi,
>
> As recommended by Walter Harms, this version of the patch uses xasprintf()
> instead of malloc()/strcpy()/strcat(). The final size of the binary is even
> smaller: 324 bytes instead of 388 bytes with the previous patch.
>
> make bloatcheck
> function old new delta
> parse_conf 1201 1372 +171
> sendHeaders 461 571 +110
> sendFile 252 317 +65
> httpResponseNames 108 144 +36
> handleIncoming 2032 2038 +6
> .rodata 138727 138663 -64
> ------------------------------------------------------------------------------
> (add/remove: 0/0 grow/shrink: 5/1 up/down: 388/-64) Total: 324 bytes
> text data bss dec hex filename
> 636180 3124 12800 652104 9f348 busybox_old
> 636525 3288 12800 652613 9f545 busybox_unstripped
+#if ENABLE_FEATURE_HTTPD_ERROR_PAGES
+ if (flag == FIRST_PARSE && *p0 == 'E') {
+ /* error status code */
+ c = strchr(++p0, ':');
+ *c = 0;
+ int status = xatoi(p0);
+ p0 = ++c;
Will crash if there is no ':'. However, there are more of such
things in the code around, so probably don't care about that now...
Some people dislike "int status" in the middle of the block.
I think bb_strtou will give smaller code here:
status = bb_strtou(p0 + 1, &p0, 10); /* ignoring possible conv error
here */
p0++;
+ /* then error page */
+ if (status >= HTTP_CONTINUE) {
+ if (*p0 == '/') {
+ c = xasprintf("%s", p0);
Use c = xstrdup(p0).
+ } else {
+ c = xasprintf("%s/%s", home_httpd, p0);
Use "c = concat_path_file(home_httpd, p0)" instead (a bit smaller).
+ }
+
+ /* find matching status */
+ for (int i = 0; i <
ARRAY_SIZE(httpResponseNames); i++) {
+ if (httpResponseNames[i].type ==
status) {
BTW, you may do alloc here, not above (and thus you won't leak c
if user specified non-existent error code).
+ httpResponseNames[i].error_page
= c;
+ break;
+ }
+ }
+ continue;
+ }
+ }
+#endif
- "<HEAD><TITLE>%d %s</TITLE></HEAD>\n"
- "<BODY><H1>%d %s</H1>\n%s\n</BODY>\n",
+ "<!DOCTYPE HTML PUBLIC \"-//IETF//DTD HTML
2.0//EN\">\n"
+ "<HTML><HEAD><TITLE>%d %s</TITLE></HEAD>\n"
+ "<BODY><H1>%d %s</H1>\n%s\n</BODY></HTML>\n",
Well, adding <html> is _maybe_ ok, but <doctype>? Isn't that just bloat?
[to please really size-paranoid people on the list, consider just dropping it]
libbb.h
+#if ENABLE_HTTPD
+#include <sys/sendfile.h>
+#endif
I think this should just be in httpd.c
- int count;
- char *buf = iobuf;
-
- sendHeaders(HTTP_OK);
- /* TODO: sendfile() */
- while ((count = full_read(f, buf, MAX_MEMORY_BUFF)) > 0) {
- int fd = accepted_socket;
- if (fd == 0) fd++; /* write to fd# 1 in inetd mode */
- if (full_write(fd, buf, count) != count)
- break;
+ if (headers) {
+ sendHeaders(HTTP_OK);
}
+
+ int fd = accepted_socket;
+ if (fd == 0) fd++; /* write to fd #1 in inetd mode */
+
+ struct stat f_stat;
+ if (fstat(f, &f_stat)) {
+ bb_perror_msg("cannot stat file '%s'", url);
+ }
+ off_t offset = 0;
+ sendfile(fd, f, &offset, f_stat.st_size);
Thanks for the effort of adding support for sendfile, but some people may want
to exclude that from kernel & bbox. Can you make it configurable?
Or just drop for now, to not interfere with inclusion of "error pages" patch.
Waiting for patch v3.
--
vda
_______________________________________________
busybox mailing list
[email protected]
http://busybox.net/cgi-bin/mailman/listinfo/busybox
