[PATCH 1/2] wget: replace set_alarm with non blocking functions to support retries
Signed-off-by: Martin Lewis --- networking/wget.c | 162 +++--- 1 file changed, 153 insertions(+), 9 deletions(-) diff --git a/networking/wget.c b/networking/wget.c index 58a51d9..e9fdd9f 100644 --- a/networking/wget.c +++ b/networking/wget.c @@ -355,6 +355,8 @@ static char *base64enc(const char *str) } #endif +/* We need to find another way to handle timeouts in order to support retries */ +#if 0 #if ENABLE_FEATURE_WGET_TIMEOUT static void alarm_handler(int sig UNUSED_PARAM) { @@ -374,6 +376,7 @@ static void set_alarm(void) # define set_alarm() ((void)0) # define clear_alarm() ((void)0) #endif +#endif #if ENABLE_FEATURE_WGET_OPENSSL /* @@ -403,10 +406,16 @@ static FILE *open_socket(len_and_sockaddr *lsa) { int fd; FILE *fp; - - set_alarm(); - fd = xconnect_stream(lsa); - clear_alarm(); +#if ENABLE_FEATURE_WGET_TIMEOUT + struct timeval timeout = {G.timeout_seconds, 0}; +#endif + fd = xsocket(lsa->u.sa.sa_family, SOCK_STREAM, 0); +#if ENABLE_FEATURE_WGET_TIMEOUT + if (setsockopt(fd, SOL_SOCKET, SO_RCVTIMEO, &timeout, sizeof (timeout)) < 0) { + bb_perror_msg_and_die("setsockopt failed\n"); + } +#endif + xconnect(fd, &lsa->u.sa, lsa->len); /* glibc 2.4 seems to try seeking on it - ??! */ /* hopefully it understands what ESPIPE means... */ @@ -439,16 +448,150 @@ static char* sanitize_string(char *s) return s; } +/* fgets() is blocking, so we need a non blocking version. +We need to do some buffering so we can cut at \n */ +static char fgets_buffer[4096]; +size_t fgets_buffer_len = 0; + +/* Returns buffer if successfull, otherswise return NULL. Supports timeout. */ +static char *fgets_read_to_newline(char *buffer, size_t len, FILE *fp) +{ + struct pollfd polldata; + size_t bytes_read = 0; + char *newline = NULL; + char *buffer_start = buffer; + + polldata.fd = fileno(fp); + polldata.events = POLLIN | POLLPRI; + +#if ENABLE_FEATURE_WGET_TIMEOUT + ndelay_on(polldata.fd); +#endif + + if (len < 1) { + /* Should not happen */ + return NULL; + } + + /* Clear buffer */ + if (fgets_buffer_len > 0) { + if ((newline = memchr(fgets_buffer, '\n', sizeof (fgets_buffer { + size_t to_copy = 0; + /* In the buffer there is a complete line, if we can copy and return */ + if ((newline + 1 - fgets_buffer) + 1 <= len) + to_copy = newline + 1 - fgets_buffer; + else + /* Otherwise we copy only part of it */ + to_copy = len - 1; + + memcpy(buffer, fgets_buffer, to_copy); + /* Comply fgets we are replacing */ + buffer[to_copy] = '\0'; + fgets_buffer_len -= to_copy; + memmove(fgets_buffer, fgets_buffer + to_copy, fgets_buffer_len); + + return buffer_start; + } + + /* We don't have \n yet, empty the buffer */ + if (fgets_buffer_len <= len - 1) { + memcpy(buffer, fgets_buffer, fgets_buffer_len); + len -= fgets_buffer_len; + buffer += fgets_buffer_len; + fgets_buffer_len = 0; + } else { + /* Or if it is not big enough */ + memcpy(buffer, fgets_buffer, len - 1); + /* Comply fgets we are replacing */ + buffer[len - 1] = '\0'; + fgets_buffer_len -= len - 1; + memmove(fgets_buffer, fgets_buffer + len - 1, fgets_buffer_len); + + return buffer_start; + } + } + + while (1) { + /* Check if data available */ +#if ENABLE_FEATURE_WGET_TIMEOUT + if (safe_poll(&polldata, 1, G.timeout_seconds * 1000) == 0) { +#else + /* Infinite time */ + if (safe_poll(&polldata, 1, -1) == 0) { +#endif + /* Timed out */ + return NULL; + } + + bytes_read = fread(fgets_buffer + fgets_buffer_len, 1, sizeof (fgets_buffer) - fgets_buffer_len, fp); + if (bytes_read <= 0) { + return NULL; + } + + fgets_buffer_len += bytes_read; + newline = memchr(fgets_buffer, '\n', fgets_buffer_len); + if (newline != NULL) { + size_t to_copy = 0; + /* In the buffer there is a complete line,
[PATCH 2/2] wget: add support for retries in http requests
Replace die handlers with error returning so download_one_url can retry from the beginning. When retries is 1 (default) the behaviour should be the same as before. Signed-off-by: Martin Lewis --- networking/wget.c | 120 ++ 1 file changed, 95 insertions(+), 25 deletions(-) diff --git a/networking/wget.c b/networking/wget.c index e9fdd9f..4b1961c 100644 --- a/networking/wget.c +++ b/networking/wget.c @@ -123,14 +123,14 @@ //usage:#define wget_trivial_usage //usage: IF_FEATURE_WGET_LONG_OPTIONS( //usage: "[-c|--continue] [--spider] [-q|--quiet] [-O|--output-document FILE]\n" -//usage: " [--header 'header: value'] [-Y|--proxy on/off] [-P DIR]\n" +//usage: " [--header 'header: value'] [-Y|--proxy on/off] [-t|--tries TRIES] [-P DIR]\n" /* Since we ignore these opts, we don't show them in --help */ /* //usage:" [--no-check-certificate] [--no-cache] [--passive-ftp] [-t TRIES]" */ /* //usage:" [-nv] [-nc] [-nH] [-np]" */ //usage: " [-S|--server-response] [-U|--user-agent AGENT]" IF_FEATURE_WGET_TIMEOUT(" [-T SEC]") " URL..." //usage: ) //usage: IF_NOT_FEATURE_WGET_LONG_OPTIONS( -//usage: "[-cq] [-O FILE] [-Y on/off] [-P DIR] [-S] [-U AGENT]" +//usage: "[-cq] [-O FILE] [-Y on/off] [-t TRIES] [-P DIR] [-S] [-U AGENT]" //usage: IF_FEATURE_WGET_TIMEOUT(" [-T SEC]") " URL..." //usage: ) //usage:#define wget_full_usage "\n\n" @@ -149,6 +149,7 @@ //usage: "\n -O FILE Save to FILE ('-' for stdout)" //usage: "\n -U STR Use STR for User-Agent header" //usage: "\n -Y on/off Use proxy" +//usage: "\n -t TRIESSet number of retries to TRIES (0 unlimits)" #include "libbb.h" @@ -233,6 +234,7 @@ struct globals { char *fname_out;/* where to direct output (-O) */ const char *proxy_flag; /* Use proxies if env vars are set */ const char *user_agent; /* "User-Agent" header field */ + unsigned tries; /* For -t option */ int output_fd; int o_flags; #if ENABLE_FEATURE_WGET_TIMEOUT @@ -402,6 +404,7 @@ static int is_ip_address(const char *string) } #endif +/* Return NULL if connect() fails */ static FILE *open_socket(len_and_sockaddr *lsa) { int fd; @@ -409,13 +412,19 @@ static FILE *open_socket(len_and_sockaddr *lsa) #if ENABLE_FEATURE_WGET_TIMEOUT struct timeval timeout = {G.timeout_seconds, 0}; #endif + fd = xsocket(lsa->u.sa.sa_family, SOCK_STREAM, 0); #if ENABLE_FEATURE_WGET_TIMEOUT if (setsockopt(fd, SOL_SOCKET, SO_RCVTIMEO, &timeout, sizeof (timeout)) < 0) { bb_perror_msg_and_die("setsockopt failed\n"); } #endif - xconnect(fd, &lsa->u.sa, lsa->len); + if (connect(fd, &lsa->u.sa, lsa->len) < 0) { + /* Failure */ + bb_perror_msg("connect failed"); + close(fd); + return NULL; + } /* glibc 2.4 seems to try seeking on it - ??! */ /* hopefully it understands what ESPIPE means... */ @@ -584,14 +593,16 @@ static int fread_buffered(char *buffer, size_t len, FILE *fp) return fread(buffer, 1, len, fp); } -/* Returns '\n' if it was seen, else '\0'. Trims at first '\r' or '\n' */ -static char fgets_trim_sanitize(FILE *fp, const char *fmt) +/* Returns '\n' if it was seen, -1 if timeout occured, else '\0'. Trims at first '\r' or '\n' */ +static signed char fgets_trim_sanitize(FILE *fp, const char *fmt) { char c; char *buf_ptr; - if (fgets_read_to_newline(G.wget_buf, sizeof(G.wget_buf), fp) == NULL) - bb_perror_msg_and_die("error getting response"); + if (fgets_read_to_newline(G.wget_buf, sizeof(G.wget_buf), fp) == NULL) { + bb_perror_msg("error getting response"); + return -1; + } buf_ptr = strchrnul(G.wget_buf, '\n'); c = *buf_ptr; @@ -629,7 +640,9 @@ static int ftpcmd(const char *s1, const char *s2, FILE *fp) /* Read until "Nxx something" is received */ G.wget_buf[3] = 0; do { - fgets_trim_sanitize(fp, "%s\n"); + if (fgets_trim_sanitize(fp, "%s\n") == -1) { + xfunc_die(); + } } while (!isdigit(G.wget_buf[0]) || G.wget_buf[3] != ' '); G.wget_buf[3] = '\0'; @@ -734,6 +747,11 @@ static char *get_sanitized_hdr(FILE *fp) /
Re: [PATCH 2/2] wget: add support for retries in http requests
Hello, These patches add the -t (--tries) option to wget. There was an issue that wget died on errors, so I had to change some functions to return error so we are able to retry. Also I had to replace the blocking functions (and the set_alarm) with time outing functions (in first patch). In order to imitate fgets(), and not reading char by char checking for newline I had to buffer the read. I've seen that many features can be turned off in config, but as I see it, adding these #ifs would rather bloat the patch, while the default behaviour without the flag should not change. Also as for ftp retries don't work yet. If you would like me to change anything in the patches I would gladly do when I have some more time. Martin ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
[PATCH 1/2] wget: add -o flag
Signed-off-by: Martin Lewis --- networking/wget.c | 28 +--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/networking/wget.c b/networking/wget.c index 58a51d9..8da0ce8 100644 --- a/networking/wget.c +++ b/networking/wget.c @@ -128,9 +128,10 @@ /* //usage:" [--no-check-certificate] [--no-cache] [--passive-ftp] [-t TRIES]" */ /* //usage:" [-nv] [-nc] [-nH] [-np]" */ //usage: " [-S|--server-response] [-U|--user-agent AGENT]" IF_FEATURE_WGET_TIMEOUT(" [-T SEC]") " URL..." +//usage: " [-o|--output-file]\n" //usage: ) //usage: IF_NOT_FEATURE_WGET_LONG_OPTIONS( -//usage: "[-cq] [-O FILE] [-Y on/off] [-P DIR] [-S] [-U AGENT]" +//usage: "[-cq] [-O FILE] [-o FILE] [-Y on/off] [-P DIR] [-S] [-U AGENT]" //usage: IF_FEATURE_WGET_TIMEOUT(" [-T SEC]") " URL..." //usage: ) //usage:#define wget_full_usage "\n\n" @@ -147,6 +148,7 @@ //usage: "\n -T SEC Network read timeout is SEC seconds" //usage: ) //usage: "\n -O FILE Save to FILE ('-' for stdout)" +//usage: "\n -o FILE Save output to FILE ('-' for stdout)" //usage: "\n -U STR Use STR for User-Agent header" //usage: "\n -Y on/off Use proxy" @@ -231,9 +233,11 @@ struct globals { unsigned char user_headers; /* Headers mentioned by the user */ #endif char *fname_out;/* where to direct output (-O) */ + char *fname_log;/* where to direct log (-o) */ const char *proxy_flag; /* Use proxies if env vars are set */ const char *user_agent; /* "User-Agent" header field */ int output_fd; + int log_fd; int o_flags; #if ENABLE_FEATURE_WGET_TIMEOUT unsigned timeout_seconds; @@ -287,6 +291,10 @@ static void progress_meter(int flag) if (option_mask32 & WGET_OPT_QUIET) return; + /* Don't save progress to log file */ + if (G.log_fd >= 0) + return; + if (flag == PROGRESS_START) bb_progress_init(&G.pmt, G.curfile); @@ -1401,6 +1409,7 @@ int wget_main(int argc UNUSED_PARAM, char **argv) "quiet\0"No_argument "q" "server-response\0" No_argument "S" "output-document\0" Required_argument "O" + "output-file\0" Required_argument "o" "directory-prefix\0" Required_argument "P" "proxy\0"Required_argument "Y" "user-agent\0" Required_argument "U" @@ -1444,7 +1453,7 @@ IF_DESKTOP( "no-parent\0"No_argument "\xf0") #if ENABLE_FEATURE_WGET_LONG_OPTIONS #endif GETOPT32(argv, "^" - "cqSO:P:Y:U:T:+" + "cqSO:o:P:Y:U:T:+" /*ignored:*/ "t:" /*ignored:*/ "n::" /* wget has exactly four -n opts, all of which we can ignore: @@ -1459,7 +1468,7 @@ IF_DESKTOP( "no-parent\0"No_argument "\xf0") "-1" /* at least one URL */ IF_FEATURE_WGET_LONG_OPTIONS(":\xff::") /* --header is a list */ LONGOPTS - , &G.fname_out, &G.dir_prefix, + , &G.fname_out, &G.fname_log, &G.dir_prefix, &G.proxy_flag, &G.user_agent, IF_FEATURE_WGET_TIMEOUT(&G.timeout_seconds) IF_NOT_FEATURE_WGET_TIMEOUT(NULL), NULL, /* -t RETRIES */ @@ -1521,12 +1530,25 @@ IF_DESKTOP( "no-parent\0"No_argument "\xf0") G.o_flags = O_WRONLY | O_CREAT | O_TRUNC; } + G.log_fd = -1; + if (G.fname_log) { /* -o FILE ? */ + if (!LONE_DASH(G.fname_log)) { /* not -o - ? */ + /* compat with wget: -o FILE can overwrite */ + G.log_fd = xopen(G.fname_log, O_WRONLY | O_CREAT | O_TRUNC); + /* Redirect only stderr to log file, so -O - will work */ + xdup2(G.log_fd, STDERR_FILENO); + } + } + while (*argv) download_one_url(*argv++); if (G.output_fd >= 0) xclose(G.output_fd); + if (G.log_fd >= 0) + xclose(G.log_fd); + #if ENABLE_FEATURE_CLEAN_UP && ENABLE_FEATURE_WGET_LONG_OPTIONS free(G.extra_headers); #endif -- 1.9.1 ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
[PATCH 2/2] wget: notify on download begin and end
When using -o to file the progress meter is not displayed, so write that we started the download and that we finished it. Signed-off-by: Martin Lewis --- networking/wget.c | 8 1 file changed, 8 insertions(+) diff --git a/networking/wget.c b/networking/wget.c index 8da0ce8..914221c 100644 --- a/networking/wget.c +++ b/networking/wget.c @@ -876,6 +876,10 @@ static void NOINLINE retrieve_file_data(FILE *dfp) polldata.fd = fileno(dfp); polldata.events = POLLIN | POLLPRI; #endif + if (G.output_fd == 1) + fprintf(stderr, "writing to stdout\n"); + else + fprintf(stderr, "saving to '%s'\n", G.fname_out); progress_meter(PROGRESS_START); if (G.chunked) @@ -1021,6 +1025,10 @@ static void NOINLINE retrieve_file_data(FILE *dfp) G.chunked = 0; /* makes it show 100% even for chunked download */ G.got_clen = 1; /* makes it show 100% even for download of (formerly) unknown size */ progress_meter(PROGRESS_END); + if (G.output_fd == 1) + fprintf(stderr, "written to stdout\n"); + else + fprintf(stderr, "'%s' saved\n", G.fname_out); } static void download_one_url(const char *url) -- 1.9.1 ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH 2/2] wget: notify on download begin and end
Thank you I have noticed that mistakenly the last patch doesn't respect the quiet option, so I'm sending a fix On Fri, 4 Jan 2019 at 18:28, Denys Vlasenko wrote: > On Wed, Dec 26, 2018 at 4:28 PM Martin Lewis > wrote: > > When using -o to file the progress meter is not displayed, so write that > > we started the download and that we finished it. > > Applied both patches with some fixes. > ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
[PATCH 1/1] wget: don't notify on download begin and end if quiet
When printing notification on download start and end, mistakenly, it didn't respect the quiet option Signed-off-by: Martin Lewis --- networking/wget.c | 23 ++- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/networking/wget.c b/networking/wget.c index 3a02de6..735746e 100644 --- a/networking/wget.c +++ b/networking/wget.c @@ -876,10 +876,12 @@ static void NOINLINE retrieve_file_data(FILE *dfp) polldata.fd = fileno(dfp); polldata.events = POLLIN | POLLPRI; #endif - if (G.output_fd == 1) - fprintf(stderr, "writing to stdout\n"); - else - fprintf(stderr, "saving to '%s'\n", G.fname_out); + if (!(option_mask32 & WGET_OPT_QUIET)) { + if (G.output_fd == 1) + fprintf(stderr, "writing to stdout\n"); + else + fprintf(stderr, "saving to '%s'\n", G.fname_out); + } progress_meter(PROGRESS_START); if (G.chunked) @@ -1025,10 +1027,12 @@ static void NOINLINE retrieve_file_data(FILE *dfp) G.chunked = 0; /* makes it show 100% even for chunked download */ G.got_clen = 1; /* makes it show 100% even for download of (formerly) unknown size */ progress_meter(PROGRESS_END); - if (G.output_fd == 1) - fprintf(stderr, "written to stdout\n"); - else - fprintf(stderr, "'%s' saved\n", G.fname_out); + if (!(option_mask32 & WGET_OPT_QUIET)) { + if (G.output_fd == 1) + fprintf(stderr, "written to stdout\n"); + else + fprintf(stderr, "'%s' saved\n", G.fname_out); + } } static void download_one_url(const char *url) @@ -1389,7 +1393,8 @@ However, in real world it was observed that some web servers G.output_fd = -1; } } else { - fprintf(stderr, "remote file exists\n"); + if (!(option_mask32 & WGET_OPT_QUIET)) + fprintf(stderr, "remote file exists\n"); } if (dfp != sfp) { -- 1.9.1 ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
[PATCH v2 2/2] wget: add support for retries in http requests
Replace die handlers with error returning so download_one_url can retry from the beginning. When retries is 1 (default) the behaviour should be the same as before. v2: updated diff to mind the -o option, prettified help Signed-off-by: Martin Lewis --- networking/wget.c | 125 ++ 1 file changed, 98 insertions(+), 27 deletions(-) diff --git a/networking/wget.c b/networking/wget.c index d11b201..b41f8ed 100644 --- a/networking/wget.c +++ b/networking/wget.c @@ -125,13 +125,14 @@ //usage: "[-c|--continue] [--spider] [-q|--quiet] [-O|--output-document FILE]\n" //usage: "[-o|--output-file FILE] [--header 'header: value'] [-Y|--proxy on/off]\n" /* Since we ignore these opts, we don't show them in --help */ -/* //usage:"[--no-check-certificate] [--no-cache] [--passive-ftp] [-t TRIES]" */ +/* //usage:"[--no-check-certificate] [--no-cache] [--passive-ftp]" */ /* //usage:"[-nv] [-nc] [-nH] [-np]" */ -//usage: "[-P DIR] [-S|--server-response] [-U|--user-agent AGENT]" IF_FEATURE_WGET_TIMEOUT(" [-T SEC]") " URL..." +//usage: "[-t|--tries TRIES] [-P DIR] [-S|--server-response]\n" +//usage: "[-U|--user-agent AGENT]" IF_FEATURE_WGET_TIMEOUT(" [-T SEC]") " URL..." //usage:) //usage:IF_NOT_FEATURE_WGET_LONG_OPTIONS( -//usage: "[-cq] [-O FILE] [-o FILE] [-Y on/off] [-P DIR] [-S] [-U AGENT]" -//usage:IF_FEATURE_WGET_TIMEOUT(" [-T SEC]") " URL..." +//usage: "[-cq] [-O FILE] [-o FILE] [-Y on/off] [-P DIR] [-S] [-U AGENT]\n" +//usage: "[-t TRIES]" IF_FEATURE_WGET_TIMEOUT(" [-T SEC]") " URL..." //usage:) //usage:#define wget_full_usage "\n\n" //usage: "Retrieve files via HTTP or FTP\n" @@ -150,6 +151,7 @@ //usage: "\n-o FILELog messages to FILE" //usage: "\n-U STRUse STR for User-Agent header" //usage: "\n-Y on/offUse proxy" +//usage: "\n-t TRIESSet number of retries to TRIES (0 unlimits)" #include "libbb.h" @@ -235,6 +237,7 @@ struct globals { char *fname_log;/* where to direct log (-o) */ const char *proxy_flag; /* Use proxies if env vars are set */ const char *user_agent; /* "User-Agent" header field */ +unsigned tries; /* For -t option */ int output_fd; int log_fd; int o_flags; @@ -410,6 +413,7 @@ static int is_ip_address(const char *string) } #endif +/* Return NULL if connect() fails */ static FILE *open_socket(len_and_sockaddr *lsa) { int fd; @@ -417,13 +421,19 @@ static FILE *open_socket(len_and_sockaddr *lsa) #if ENABLE_FEATURE_WGET_TIMEOUT struct timeval timeout = {G.timeout_seconds, 0}; #endif + fd = xsocket(lsa->u.sa.sa_family, SOCK_STREAM, 0); #if ENABLE_FEATURE_WGET_TIMEOUT if (setsockopt(fd, SOL_SOCKET, SO_RCVTIMEO, &timeout, sizeof (timeout)) < 0) { bb_perror_msg_and_die("setsockopt failed\n"); } #endif -xconnect(fd, &lsa->u.sa, lsa->len); +if (connect(fd, &lsa->u.sa, lsa->len) < 0) { +/* Failure */ +bb_perror_msg("connect failed"); +close(fd); +return NULL; +} /* glibc 2.4 seems to try seeking on it - ??! */ /* hopefully it understands what ESPIPE means... */ @@ -592,14 +602,16 @@ static int fread_buffered(char *buffer, size_t len, FILE *fp) return fread(buffer, 1, len, fp); } -/* Returns '\n' if it was seen, else '\0'. Trims at first '\r' or '\n' */ -static char fgets_trim_sanitize(FILE *fp, const char *fmt) +/* Returns '\n' if it was seen, -1 if timeout occured, else '\0'. Trims at first '\r' or '\n' */ +static signed char fgets_trim_sanitize(FILE *fp, const char *fmt) { char c; char *buf_ptr; -if (fgets_read_to_newline(G.wget_buf, sizeof(G.wget_buf), fp) == NULL) -bb_perror_msg_and_die("error getting response"); +if (fgets_read_to_newline(G.wget_buf, sizeof(G.wget_buf), fp) == NULL) { +bb_perror_msg("error getting response"); +return -1; +} buf_ptr = strchrnul(G.wget_buf, '\n'); c = *buf_ptr; @@ -637,7 +649,9 @@ static int ftpcmd(const char *s1, const char *s2, FILE *fp) /* Read until "Nxx something" is received */ G.wget_buf[3] = 0; do { -fgets_trim_sanitize(fp, "%s\n"); +if (fgets_trim_sanitize(fp, "%s\n") == -1) { +xfunc_die(); +} } while (!isdigit(G.wget_buf[0]) || G.wget_buf[3] != ' '); G.wget_buf[3] =
Re: [PATCH 1/2] wget: replace set_alarm with non blocking functions to support retries
Hi Denys, Thank you for the review. I moved the buffer to G, and added an incremental sleep between the retries (as in GNU wget). Even if we use poll() and read(), if we don't want to read byte by byte we need to use a buffer (like fgets does), as if we read two newlines at once we need to parse the buffer twice. As for the default number of retries, if it won't be 1 that might cause problems with already written scripts that assume wget doesn't try 20 times. Also, using 'retries' as the variable name might be a little confusing as when 'retries' = 1 it won't retry the download at all. Martin On Sun, 20 Jan 2019 at 18:46, Denys Vlasenko wrote: > Throws a warning: > > networking/wget.c: In function 'fread_buffered': > networking/wget.c:575: error: declaration of 'read' shadows a global > declaration > > Way too large, and adds to bss: > > function old new delta > fgets_buffer -4096 +4096 > fgets_trim_sanitize 128 621+493 > retrieve_file_data 579 775+196 > open_socket 49 117 +68 > fgets_buffer_len - 4 +4 > wget_main 24372435 -2 > set_alarm 27 - -27 > > -- > (add/remove: 2/1 grow/shrink: 3/1 up/down: 4857/-29) Total: 4828 > bytes >text databssdechexfilename > 979050485 7296 986831 f0ecfbusybox_old > 979775485 11400 991660 f21acbusybox_unstripped > > The second patch is mangled by gmail, please resend as attachment. > > I played with a version where one try is done in a child, > see attached z.diff. > > function old new delta > download_one_url -2221 +2221 > retrieve_file_data 579 602 +23 > ftpcmd 133 151 +18 > get_sanitized_hdr156 162 +6 > fgets_trim_sanitize 128 131 +3 > base64enc 46 49 +3 > packed_usage 33070 33042 -28 > wget_main 2437 565 -1872 > > -- > (add/remove: 1/0 grow/shrink: 5/2 up/down: 2274/-1900)Total: 374 > bytes > > I'm not entirely happy with this approach either... > ...probably need to rewrite existing code to get rid of fgets(), > use poll() + read(). > From 3d70e006affde33a9b278b2a62c81d4eb40f2394 Mon Sep 17 00:00:00 2001 From: Martin Lewis Date: Wed, 23 Jan 2019 15:46:16 +0100 Subject: [PATCH 1/2] wget: replace set_alarm with non blocking functions to support retries --- networking/wget.c | 161 +++--- 1 file changed, 152 insertions(+), 9 deletions(-) diff --git a/networking/wget.c b/networking/wget.c index fa4d21a..3df8e74 100644 --- a/networking/wget.c +++ b/networking/wget.c @@ -249,6 +249,8 @@ struct globals { * an order of magnitude slower than with big one. */ char wget_buf[CONFIG_FEATURE_COPYBUF_KB*1024] ALIGNED(16); + char fgets_buffer[4096]; + size_t fgets_buffer_len; } FIX_ALIASING; #define G (*ptr_to_globals) #define INIT_G() do { \ @@ -363,6 +365,8 @@ static char *base64enc(const char *str) } #endif +/* We need to find another way to handle timeouts in order to support retries */ +#if 0 #if ENABLE_FEATURE_WGET_TIMEOUT static void alarm_handler(int sig UNUSED_PARAM) { @@ -382,6 +386,7 @@ static void set_alarm(void) # define set_alarm() ((void)0) # define clear_alarm() ((void)0) #endif +#endif #if ENABLE_FEATURE_WGET_OPENSSL /* @@ -408,10 +413,16 @@ static FILE *open_socket(len_and_sockaddr *lsa) { int fd; FILE *fp; - - set_alarm(); - fd = xconnect_stream(lsa); - clear_alarm(); +#if ENABLE_FEATURE_WGET_TIMEOUT + struct timeval timeout = {G.timeout_seconds, 0}; +#endif + fd = xsocket(lsa->u.sa.sa_family, SOCK_STREAM, 0); +#if ENABLE_FEATURE_WGET_TIMEOUT + if (setsockopt(fd, SOL_SOCKET, SO_RCVTIMEO, &timeout, sizeof (timeout)) < 0) { + bb_perror_msg_and_die("setsockopt failed\n"); + } +#endif + xconnect(fd, &lsa->u.sa, lsa->len); /* glibc 2.4 seems to try seeking on it - ??! */ /* hopefully it underst
[PATCH] telnetd: Added support for AYT IAC command.
Fixed a TODO in AYT IAC handling by replying back with a NOP. Signed-off-by: Martin Lewis --- networking/telnetd.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/networking/telnetd.c b/networking/telnetd.c index caef151..c6fe815 100644 --- a/networking/telnetd.c +++ b/networking/telnetd.c @@ -249,7 +249,7 @@ safe_write_to_pty_decode_iac(struct tsession *ts) * IAC SE (240) End of subnegotiation. Treated as NOP. * IAC NOP (241) NOP. Supported. * IAC BRK (243) Break. Like serial line break. TODO via tcsendbreak()? -* IAC AYT (246) Are you there. Send back evidence that AYT was seen. TODO (send NOP back)? +* IAC AYT (246) Are you there. * These don't look useful: * IAC DM (242) Data mark. What is this? * IAC IP (244) Suspend, interrupt or abort the process. (Ancient cousin of ^C). @@ -277,6 +277,12 @@ safe_write_to_pty_decode_iac(struct tsession *ts) rc = 2; goto update_and_return; } + if (buf[1] == AYT) { + /* Send back evidence that AYT was seen. */ + buf[1] = NOP; + rc = safe_write(ts->sockfd_write, buf, 2); + goto update_and_return; + } if (buf[1] >= 240 && buf[1] <= 249) { /* NOP (241). Ignore (putty keepalive, etc) */ /* All other 2-byte commands also treated as NOPs here */ -- 1.9.1 ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] telnetd: Added support for AYT IAC command.
You're absolutely right, I'll fix this right away. On Thu, 4 Apr 2019 at 15:46, Denys Vlasenko wrote: > On Thu, Apr 4, 2019 at 1:33 PM Martin Lewis > wrote: > > > > Fixed a TODO in AYT IAC handling by replying back with a NOP. > > > > Signed-off-by: Martin Lewis > > --- > > networking/telnetd.c | 8 +++- > > 1 file changed, 7 insertions(+), 1 deletion(-) > > > > diff --git a/networking/telnetd.c b/networking/telnetd.c > > index caef151..c6fe815 100644 > > --- a/networking/telnetd.c > > +++ b/networking/telnetd.c > > @@ -249,7 +249,7 @@ safe_write_to_pty_decode_iac(struct tsession *ts) > > * IAC SE (240) End of subnegotiation. Treated as NOP. > > * IAC NOP (241) NOP. Supported. > > * IAC BRK (243) Break. Like serial line break. TODO via > tcsendbreak()? > > -* IAC AYT (246) Are you there. Send back evidence that AYT was > seen. TODO (send NOP back)? > > +* IAC AYT (246) Are you there. > > * These don't look useful: > > * IAC DM (242) Data mark. What is this? > > * IAC IP (244) Suspend, interrupt or abort the process. > (Ancient cousin of ^C). > > @@ -277,6 +277,12 @@ safe_write_to_pty_decode_iac(struct tsession *ts) > > rc = 2; > > goto update_and_return; > > } > > + if (buf[1] == AYT) { > > + /* Send back evidence that AYT was seen. */ > > + buf[1] = NOP; > > + rc = safe_write(ts->sockfd_write, buf, 2); > > + goto update_and_return; > > what if rc == -1 now? > > > + } > > if (buf[1] >= 240 && buf[1] <= 249) { > > /* NOP (241). Ignore (putty keepalive, etc) */ > > /* All other 2-byte commands also treated as NOPs here */ > > -- > > 1.9.1 > > > > ___ > > busybox mailing list > > busybox@busybox.net > > http://lists.busybox.net/mailman/listinfo/busybox > ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
[PATCH 1/1] [PATCH] udhcp: Fixed aliasing compilation error and alignment issues.
Fixed the compilation warning: networking/udhcp/d6_common.h:146:99: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing] #define client6_data (*(struct client6_data_t*)(&bb_common_bufsiz1[COMMON_BUFSIZE - sizeof(struct client6_data_t)])) ^ Also fixed a possible alignment issue related to strict aliasing. Signed-off-by: Martin Lewis --- networking/udhcp/d6_common.h | 2 +- networking/udhcp/d6_dhcpc.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/networking/udhcp/d6_common.h b/networking/udhcp/d6_common.h index 2178cb9..dee2558 100644 --- a/networking/udhcp/d6_common.h +++ b/networking/udhcp/d6_common.h @@ -141,7 +141,7 @@ struct client6_data_t { unsigned env_idx; /* link-local IPv6 address */ struct in6_addr ll_ip6; -}; +} FIX_ALIASING; #define client6_data (*(struct client6_data_t*)(&bb_common_bufsiz1[COMMON_BUFSIZE - sizeof(struct client6_data_t)])) diff --git a/networking/udhcp/d6_dhcpc.c b/networking/udhcp/d6_dhcpc.c index 6cc2316..e424ce6 100644 --- a/networking/udhcp/d6_dhcpc.c +++ b/networking/udhcp/d6_dhcpc.c @@ -643,7 +643,7 @@ static NOINLINE int send_d6_discover(uint32_t xid, struct in6_addr *requested_ip client6_data.ia_na = xzalloc(len); client6_data.ia_na->code = D6_OPT_IA_NA; client6_data.ia_na->len = len - 4; - *(uint32_t*)client6_data.ia_na->data = rand(); /* IAID */ + move_to_unaligned32(client6_data.ia_na->data, rand()); if (requested_ipv6) { struct d6_option *iaaddr = (void*)(client6_data.ia_na->data + 4+4+4); iaaddr->code = D6_OPT_IAADDR; @@ -661,7 +661,7 @@ static NOINLINE int send_d6_discover(uint32_t xid, struct in6_addr *requested_ip client6_data.ia_pd = xzalloc(len); client6_data.ia_pd->code = D6_OPT_IA_PD; client6_data.ia_pd->len = len - 4; - *(uint32_t*)client6_data.ia_pd->data = rand(); /* IAID */ + move_to_unaligned32(client6_data.ia_pd->data, rand()); opt_ptr = mempcpy(opt_ptr, client6_data.ia_pd, len); } -- 1.9.1 ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
[PATCH] dhcpc.c: Added support for relay server parameter.
Resolved a TODO by adding support for gateway_nip parameter. Signed-off-by: Martin Lewis --- networking/udhcp/dhcpc.c | 13 + 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/networking/udhcp/dhcpc.c b/networking/udhcp/dhcpc.c index 739870b..acd80f9 100644 --- a/networking/udhcp/dhcpc.c +++ b/networking/udhcp/dhcpc.c @@ -449,15 +449,16 @@ static char **fill_envp(struct dhcp_packet *packet) memset(found_opts, 0, sizeof(found_opts)); - /* We need 6 elements for: + /* We need 7 elements for: * "interface=IFACE" * "ip=N.N.N.N" from packet->yiaddr + * "giaddr=IP" from packet->gateway_nip (unless 0) * "siaddr=IP" from packet->siaddr_nip (unless 0) * "boot_file=FILE" from packet->file (unless overloaded) * "sname=SERVER_HOSTNAME" from packet->sname (unless overloaded) * terminating NULL */ - envc = 6; + envc = 7; /* +1 element for each option, +2 for subnet option: */ if (packet) { /* note: do not search for "pad" (0) and "end" (255) options */ @@ -493,9 +494,7 @@ static char **fill_envp(struct dhcp_packet *packet) * uint16_t flags; // only one flag so far: bcast. Never set by server * uint32_t ciaddr; // client IP (usually == yiaddr. can it be different * // if during renew server wants to give us different IP?) -* uint32_t gateway_nip; // relay agent IP address * uint8_t chaddr[16]; // link-layer client hardware address (MAC) -* TODO: export gateway_nip as $giaddr? */ /* Most important one: yiaddr as $ip */ *curr = xmalloc(sizeof("ip=255.255.255.255")); @@ -507,6 +506,12 @@ static char **fill_envp(struct dhcp_packet *packet) sprint_nip(*curr, "siaddr=", (uint8_t *) &packet->siaddr_nip); putenv(*curr++); } + if (packet->gateway_nip) { + /* IP address of DHCP relay agent to use in bootstrap */ + *curr = xmalloc(sizeof("giaddr=255.255.255.255")); + sprint_nip(*curr, "giaddr=", (uint8_t *) &packet->gateway_nip); + putenv(*curr++); + } if (!(overload & FILE_FIELD) && packet->file[0]) { /* watch out for invalid packets */ *curr = xasprintf("boot_file=%."DHCP_PKT_FILE_LEN_STR"s", packet->file); -- 1.9.1 ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
[PATCH] telnet: added support for special telnet characters
Closes https://bugs.busybox.net/show_bug.cgi?id=11976 Adds a new flag in telnet client's command line, which forces telnet to send IAC's instead of the traditional special characters (backspace, newline etc..). This code is similar to putty's code, and is added for compatibility with older telnet servers. Currently only backspace is supported, I will add support for other characters soon. Signed-off-by: Martin Lewis --- networking/telnet.c | 40 ++-- 1 file changed, 34 insertions(+), 6 deletions(-) diff --git a/networking/telnet.c b/networking/telnet.c index fa16287..b6bebdc 100644 --- a/networking/telnet.c +++ b/networking/telnet.c @@ -120,6 +120,7 @@ struct globals { bytecharmode; bytetelflags; bytedo_termios; + bytekeyboard_special_cmds; #if ENABLE_FEATURE_TELNET_TTYPE char*ttype; #endif @@ -175,6 +176,7 @@ static void con_escape(void) full_write1_str("\r\nConsole escape. Commands are:\r\n\n" " l go to line mode\r\n" " c go to character mode\r\n" + " k toggle keyboard sends telnet special commands\r\n" " z suspend telnet\r\n" " e exit telnet\r\n"); @@ -194,6 +196,9 @@ static void con_escape(void) goto ret; } break; + case 'k': + G.keyboard_special_cmds = !G.keyboard_special_cmds; + break; case 'z': cookmode(); kill(0, SIGTSTP); @@ -220,15 +225,16 @@ static void handle_net_output(int len) while (src < end) { byte c = *src++; - if (c == 0x1d) { + switch (c) { + case 0x1d: con_escape(); return; - } - *dst = c; - if (c == IAC) + case IAC: + *dst = c; *++dst = c; /* IAC -> IAC IAC */ - else - if (c == '\r' || c == '\n') { + break; + case '\r': + case '\n': /* Enter key sends '\r' in raw mode and '\n' in cooked one. * * See RFC 1123 3.3.1 Telnet End-of-Line Convention. @@ -237,6 +243,28 @@ static void handle_net_output(int len) */ *dst = '\r'; /* Enter -> CR LF */ *++dst = '\n'; + break; + + case 0x08: /* ctrl+h */ + case 0x7f: /* backspace */ + if (G.keyboard_special_cmds) { + *dst = IAC; + *++dst = EC; + break; + } + /* fall through */ + + case 0x03: /* ctrl+c */ + if (G.keyboard_special_cmds) { + *dst = IAC; + *++dst = IP; + break; + } + /* fall through */ + + default: + *dst = c; + break; } dst++; } -- 1.9.1 ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
[PATCH] arpping.c: Fixed a TODO by adding an error message print
Signed-off-by: Martin Lewis --- networking/udhcp/arpping.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/networking/udhcp/arpping.c b/networking/udhcp/arpping.c index a395e83..59c2bf8 100644 --- a/networking/udhcp/arpping.c +++ b/networking/udhcp/arpping.c @@ -80,8 +80,7 @@ int FAST_FUNC arpping(uint32_t test_nip, memset(&addr, 0, sizeof(addr)); safe_strncpy(addr.sa_data, interface, sizeof(addr.sa_data)); if (sendto(s, &arp, sizeof(arp), 0, &addr, sizeof(addr)) < 0) { - // TODO: error message? caller didn't expect us to fail, - // just returning 1 "no reply received" misleads it. + bb_perror_msg("can't send arp request on raw socket"); goto ret; } -- 1.9.1 ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
[PATCH] brctl: Added support for showmacs command
Signed-off-by: Martin Lewis --- networking/brctl.c | 119 - 1 file changed, 100 insertions(+), 19 deletions(-) diff --git a/networking/brctl.c b/networking/brctl.c index 586ca9b..35771a5 100644 --- a/networking/brctl.c +++ b/networking/brctl.c @@ -61,10 +61,10 @@ //usage: "\n setbridgeprio BRIDGE PRIO Set bridge priority" //usage: "\n setportprio BRIDGE IFACE PRIO Set port priority" //usage: "\n setpathcost BRIDGE IFACE COST Set path cost" +//usage: "\n showmacs BRIDGE List mac addrs" //usage: ) // Not yet implemented: // hairpin BRIDGE IFACE on|off Hairpin on/off -// showmacs BRIDGE List mac addrs // showstp BRIDGE Show stp info #include "libbb.h" @@ -196,6 +196,94 @@ static void write_uint(const char *name, const char *leaf, unsigned val) bb_simple_perror_msg_and_die(name); close(fd); } + +struct __fdb_entry { + uint8_t mac_addr[6]; + uint8_t port_no; + uint8_t is_local; + uint32_t ageing_timer_value; + uint8_t port_hi; + uint8_t pad0; + uint16_t unused; +}; + +static int compare_fdbs(const void *_f0, const void *_f1) +{ + const struct __fdb_entry *f0 = _f0; + const struct __fdb_entry *f1 = _f1; + + return memcmp(f0->mac_addr, f1->mac_addr, 6); +} + +static int read_bridge_forward_db(const char *name, struct __fdb_entry **_fdb, size_t *_nentries) +{ + struct __fdb_entry *fdb = NULL; + size_t nentries = 0; + + char *path; + int fd; + ssize_t cc; + + path = concat_path_file(name, "/brforward"); + fd = open(path, O_RDONLY); + free(path); + if (fd < 0) + return -1; + + for (;;) { + fdb = xrealloc(fdb, (nentries + 1) * sizeof(*fdb)); + cc = full_read(fd, &fdb[nentries], sizeof(*fdb)); + if (cc < 0) { + bb_perror_msg_and_die("can't read bridge %s forward db", name); + } + if (cc == 0) { + break; + } + ++nentries; + } + + close(fd); + + qsort(fdb, nentries, sizeof(*fdb), compare_fdbs); + + *_fdb = fdb; + *_nentries = nentries; + return 0; +} + +static inline void show_bridge_timer(uint32_t ageing_timer) +{ + unsigned long long tvusec = 1ULL * ageing_timer; + unsigned int tv_sec = tvusec / 100; + unsigned int tv_usec = tvusec - (100 * tv_sec); + + printf("%4u.%.2u", tv_sec, tv_usec / 1); +} + +static int show_bridge_macs(const char *name) +{ + struct __fdb_entry *fdb; + size_t nentries; + size_t i; + + if (read_bridge_forward_db(name, &fdb, &nentries) < 0) + return -1; + + printf("port no\tmac addr\t\tis local?\tageing timer\n"); + for (i = 0; i < nentries; ++i) { + const struct __fdb_entry *f = &fdb[i]; + printf("%3u\t", f->port_no); + printf("%.2x:%.2x:%.2x:%.2x:%.2x:%.2x\t", + f->mac_addr[0], f->mac_addr[1], f->mac_addr[2], + f->mac_addr[3], f->mac_addr[4], f->mac_addr[5]); + printf("%s\t\t", f->is_local ? "yes" : "no"); + show_bridge_timer(f->ageing_timer_value); + printf("\n"); + } + + free(fdb); + return 0; +} #endif int brctl_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE; @@ -208,6 +296,7 @@ int brctl_main(int argc UNUSED_PARAM, char **argv) "setageing\0" "setfd\0" "sethello\0" "setmaxage\0" "setpathcost\0" "setportprio\0" "setbridgeprio\0" + "showmacs\0" ) IF_FEATURE_BRCTL_SHOW("show\0"); enum { ARG_addbr = 0, ARG_delbr, ARG_addif, ARG_delif @@ -215,7 +304,8 @@ int brctl_main(int argc UNUSED_PARAM, char **argv) ARG_stp, ARG_setageing, ARG_setfd, ARG_sethello, ARG_setmaxage, ARG_setpathcost, ARG_setportprio, - ARG_setbridgeprio + ARG_setbridgeprio, + ARG_showmacs ) IF_FEATURE_BRCTL_SHOW(, ARG_show) }; @@ -299,6 +389,14 @@ int brctl_main(int argc UNUSED_PARAM, char **argv) return EXIT_SUCCESS; } + if (key == ARG_showmacs) { + if (show_bridge_macs(br) <
[PATCH] libbb: Converted safe_read to safe_write format
Changed safe_read to be symmetrical to safe_write, it shall never return EINTR because it calls read multiple times, the error is considered transient. Also, as seen in gnu coreutils, handle an edge case where count is bigger than INT_MAX by truncating it in order to avoid bugs on various linux platforms. Signed-off-by: Martin Lewis --- include/libbb.h| 14 ++ libbb/read.c | 15 +-- libbb/safe_write.c | 3 +++ 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/include/libbb.h b/include/libbb.h index 111d1b7..07b1d05 100644 --- a/include/libbb.h +++ b/include/libbb.h @@ -911,26 +911,40 @@ unsigned bb_clk_tck(void) FAST_FUNC; /* Autodetects gzip/bzip2 formats. fd may be in the middle of the file! */ int setup_unzip_on_fd(int fd, int fail_if_not_compressed) FAST_FUNC; /* Autodetects .gz etc */ extern int open_zipped(const char *fname, int fail_if_not_compressed) FAST_FUNC; extern void *xmalloc_open_zipped_read_close(const char *fname, size_t *maxsz_p) FAST_FUNC RETURNS_MALLOC; #else # define setup_unzip_on_fd(...) (0) # define open_zipped(fname, fail_if_not_compressed) open((fname), O_RDONLY); # define xmalloc_open_zipped_read_close(fname, maxsz_p) xmalloc_open_read_close((fname), (maxsz_p)) #endif /* lzma has no signature, need a little helper. NB: exist only for ENABLE_FEATURE_SEAMLESS_LZMA=y */ void setup_lzma_on_fd(int fd) FAST_FUNC; +/* + * Code exported from gnu coreutils: + * Maximum number of bytes to read or write in a single system call. + * This can be useful for system calls like sendfile on GNU/Linux, + * which do not handle more than MAX_RW_COUNT bytes correctly. + * The Linux kernel MAX_RW_COUNT is at least ((INT_MAX >> 20) << 20), + * where the 20 comes from the Hexagon port with 1 MiB pages; use that + * as an approximation, as the exact value may not be available to us. + * + * Using this also works around a serious Linux bug before 2.6.16; see + * <https://bugzilla.redhat.com/show_bug.cgi?id=612839>. + */ +enum { SYS_BUFSIZE_MAX = ((INT_MAX >> 20) << 20)}; + extern ssize_t safe_write(int fd, const void *buf, size_t count) FAST_FUNC; // NB: will return short write on error, not -1, // if some data was written before error occurred extern ssize_t full_write(int fd, const void *buf, size_t count) FAST_FUNC; extern void xwrite(int fd, const void *buf, size_t count) FAST_FUNC; extern void xwrite_str(int fd, const char *str) FAST_FUNC; extern ssize_t full_write1_str(const char *str) FAST_FUNC; extern ssize_t full_write2_str(const char *str) FAST_FUNC; extern void xopen_xwrite_close(const char* file, const char *str) FAST_FUNC; /* Close fd, but check for failures (some types of write errors) */ extern void xclose(int fd) FAST_FUNC; diff --git a/libbb/read.c b/libbb/read.c index 5906bc2..f91aed4 100644 --- a/libbb/read.c +++ b/libbb/read.c @@ -2,29 +2,40 @@ /* * Utility routines. * * Copyright (C) 1999-2004 by Erik Andersen * * Licensed under GPLv2 or later, see file LICENSE in this source tree. */ #include "libbb.h" ssize_t FAST_FUNC safe_read(int fd, void *buf, size_t count) { ssize_t n; - do { +if (count > SYS_BUFSIZE_MAX) +count = SYS_BUFSIZE_MAX; + + for (;;) { n = read(fd, buf, count); - } while (n < 0 && errno == EINTR); + if (n >= 0 || errno != EINTR) + break; + /* Some callers set errno=0, are upset when they see EINTR. +* Returning EINTR is wrong since we retry read(), +* the "error" was transient. +*/ + errno = 0; + /* repeat the read() */ + } return n; } /* * Read all of the supplied buffer from a file. * This does multiple reads as necessary. * Returns the amount read, or -1 on an error. * A short read is returned on an end of file. */ ssize_t FAST_FUNC full_read(int fd, void *buf, size_t len) { ssize_t cc; diff --git a/libbb/safe_write.c b/libbb/safe_write.c index 12bb438..97ceac0 100644 --- a/libbb/safe_write.c +++ b/libbb/safe_write.c @@ -2,26 +2,29 @@ /* * Utility routines. * * Copyright (C) 1999-2004 by Erik Andersen * * Licensed under GPLv2 or later, see file LICENSE in this source tree. */ #include "libbb.h" ssize_t FAST_FUNC safe_write(int fd, const void *buf, size_t count) { ssize_t n; +if (count > SYS_BUFSIZE_MAX) +count = SYS_BUFSIZE_MAX; + for (;;) { n = write(fd, buf, count); if (n >= 0 || errno != EINTR) break; /* Some callers set errno=0, are upset when they see EINTR. * Returning EINTR is wrong since we retry write(), * the "error" was transient. */
Re: [PATCH] libbb: Converted safe_read to safe_write format
Noticed just now the problematic indentation. I'll fix it in a second, sorry for the inconvenience. ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
[PATCH] libbb: Converted safe_read to safe_write format
Changed safe_read to be symmetrical to safe_write, it shall never return EINTR because it calls read multiple times, the error is considered transient. Also, as seen in gnu coreutils, handle an edge case where count is bigger than INT_MAX by truncating it in order to avoid bugs on various linux platforms. Signed-off-by: Martin Lewis --- include/libbb.h| 14 ++ libbb/read.c | 15 +-- libbb/safe_write.c | 3 +++ 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/include/libbb.h b/include/libbb.h index 111d1b7..07b1d05 100644 --- a/include/libbb.h +++ b/include/libbb.h @@ -911,26 +911,40 @@ unsigned bb_clk_tck(void) FAST_FUNC; /* Autodetects gzip/bzip2 formats. fd may be in the middle of the file! */ int setup_unzip_on_fd(int fd, int fail_if_not_compressed) FAST_FUNC; /* Autodetects .gz etc */ extern int open_zipped(const char *fname, int fail_if_not_compressed) FAST_FUNC; extern void *xmalloc_open_zipped_read_close(const char *fname, size_t *maxsz_p) FAST_FUNC RETURNS_MALLOC; #else # define setup_unzip_on_fd(...) (0) # define open_zipped(fname, fail_if_not_compressed) open((fname), O_RDONLY); # define xmalloc_open_zipped_read_close(fname, maxsz_p) xmalloc_open_read_close((fname), (maxsz_p)) #endif /* lzma has no signature, need a little helper. NB: exist only for ENABLE_FEATURE_SEAMLESS_LZMA=y */ void setup_lzma_on_fd(int fd) FAST_FUNC; +/* + * Code exported from gnu coreutils: + * Maximum number of bytes to read or write in a single system call. + * This can be useful for system calls like sendfile on GNU/Linux, + * which do not handle more than MAX_RW_COUNT bytes correctly. + * The Linux kernel MAX_RW_COUNT is at least ((INT_MAX >> 20) << 20), + * where the 20 comes from the Hexagon port with 1 MiB pages; use that + * as an approximation, as the exact value may not be available to us. + * + * Using this also works around a serious Linux bug before 2.6.16; see + * <https://bugzilla.redhat.com/show_bug.cgi?id=612839>. + */ +enum { SYS_BUFSIZE_MAX = ((INT_MAX >> 20) << 20)}; + extern ssize_t safe_write(int fd, const void *buf, size_t count) FAST_FUNC; // NB: will return short write on error, not -1, // if some data was written before error occurred extern ssize_t full_write(int fd, const void *buf, size_t count) FAST_FUNC; extern void xwrite(int fd, const void *buf, size_t count) FAST_FUNC; extern void xwrite_str(int fd, const char *str) FAST_FUNC; extern ssize_t full_write1_str(const char *str) FAST_FUNC; extern ssize_t full_write2_str(const char *str) FAST_FUNC; extern void xopen_xwrite_close(const char* file, const char *str) FAST_FUNC; /* Close fd, but check for failures (some types of write errors) */ extern void xclose(int fd) FAST_FUNC; diff --git a/libbb/read.c b/libbb/read.c index 5906bc2..65ef18f 100644 --- a/libbb/read.c +++ b/libbb/read.c @@ -2,29 +2,40 @@ /* * Utility routines. * * Copyright (C) 1999-2004 by Erik Andersen * * Licensed under GPLv2 or later, see file LICENSE in this source tree. */ #include "libbb.h" ssize_t FAST_FUNC safe_read(int fd, void *buf, size_t count) { ssize_t n; - do { + if (count > SYS_BUFSIZE_MAX) + count = SYS_BUFSIZE_MAX; + + for (;;) { n = read(fd, buf, count); - } while (n < 0 && errno == EINTR); + if (n >= 0 || errno != EINTR) + break; + /* Some callers set errno=0, are upset when they see EINTR. +* Returning EINTR is wrong since we retry read(), +* the "error" was transient. +*/ + errno = 0; + /* repeat the read() */ + } return n; } /* * Read all of the supplied buffer from a file. * This does multiple reads as necessary. * Returns the amount read, or -1 on an error. * A short read is returned on an end of file. */ ssize_t FAST_FUNC full_read(int fd, void *buf, size_t len) { ssize_t cc; diff --git a/libbb/safe_write.c b/libbb/safe_write.c index 12bb438..bfffe5b 100644 --- a/libbb/safe_write.c +++ b/libbb/safe_write.c @@ -2,26 +2,29 @@ /* * Utility routines. * * Copyright (C) 1999-2004 by Erik Andersen * * Licensed under GPLv2 or later, see file LICENSE in this source tree. */ #include "libbb.h" ssize_t FAST_FUNC safe_write(int fd, const void *buf, size_t count) { ssize_t n; + if (count > SYS_BUFSIZE_MAX) + count = SYS_BUFSIZE_MAX; + for (;;) { n = write(fd, buf, count); if (n >= 0 || errno != EINTR) break; /* Some callers set errno=0, are upset when they see EINTR. * Returning EINTR is wrong since we retry write(), * the "error" was transient. */
[PATCH] replace: count_strstr - Handle an edge case where sub is empty
If sub is empty, avoids an infinite loop. Signed-off-by: Martin Lewis --- libbb/replace.c | 4 1 file changed, 4 insertions(+) diff --git a/libbb/replace.c b/libbb/replace.c index a661d96..6183d3e 100644 --- a/libbb/replace.c +++ b/libbb/replace.c @@ -11,14 +11,18 @@ #include "libbb.h" unsigned FAST_FUNC count_strstr(const char *str, const char *sub) { size_t sub_len = strlen(sub); unsigned count = 0; + /* If sub is empty, avoid an infinite loop */ + if (sub_len == 0) + return strlen(str) + 1; + while ((str = strstr(str, sub)) != NULL) { count++; str += sub_len; } return count; } -- 1.9.1 ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] brctl: Added support for showmacs command
Hi, The attached implementation is roughly the same found in brctl from bridge-utils. It does the same actions, and specifically reads it’s entries from /sys/class/net//brforward. The relevant kernel function is brforward_read, and can be found under net/bridge/br_sysfs_br.c in the kernel source tree. All it does is provide the user an array of entries of the type struct __fdb_entry, which clearly contains a member __u8 mac_addr[ETH_ALEN], where ETH_ALEN is defined to be 6. There are no references to Infiniband or in fact non-ethernet devices in the kernel’s bridge implementation, will you be able to please provide an example / code reference in which the above code breaks (as I couldn’t find any)? Thank you, Martin On Mon, 16 Sep 2019 at 07:54, Bernhard Reutner-Fischer wrote: > > On 15 September 2019 18:04:49 CEST, Martin Lewis > wrote: > >Signed-off-by: Martin Lewis > >--- > >networking/brctl.c | 119 > >- > > 1 file changed, 100 insertions(+), 19 deletions(-) > > > >diff --git a/networking/brctl.c b/networking/brctl.c > >index 586ca9b..35771a5 100644 > >--- a/networking/brctl.c > >+++ b/networking/brctl.c > >@@ -61,10 +61,10 @@ > > //usage: "\n setbridgeprio BRIDGE PRIO Set bridge priority" > > //usage: "\n setportprio BRIDGE IFACE PRIO Set port priority" > > //usage: "\n setpathcost BRIDGE IFACE COST Set path cost" > >+//usage: "\n showmacs BRIDGE List mac addrs" > > //usage: ) > > // Not yet implemented: > > //hairpin BRIDGE IFACE on|off Hairpin on/off > >-//showmacs BRIDGE List mac addrs > > //showstp BRIDGE Show stp info > > > > #include "libbb.h" > >@@ -196,6 +196,94 @@ static void write_uint(const char *name, const > >char *leaf, unsigned val) > > bb_simple_perror_msg_and_die(name); > > close(fd); > > } > >+ > >+struct __fdb_entry { > >+ uint8_t mac_addr[6]; > > Infiniband has 20 bytes MAC addresses so this is not accurate. > > Thanks, ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
[PATCH] brctl: add support for showstp command
Signed-off-by: Martin Lewis --- networking/brctl.c | 217 - 1 file changed, 216 insertions(+), 1 deletion(-) diff --git a/networking/brctl.c b/networking/brctl.c index ba5b6226f..cb55b0c90 100644 --- a/networking/brctl.c +++ b/networking/brctl.c @@ -54,6 +54,7 @@ //usage: "\n delif BRIDGE IFACE Delete IFACE from BRIDGE" //usage: IF_FEATURE_BRCTL_FANCY( //usage: "\n stp BRIDGE 1/yes/on|0/no/offSTP on/off" +//usage: "\n showstp BRIDGE Show stp info" //usage: "\n setageing BRIDGE SECONDSSet ageing time" //usage: "\n setfd BRIDGE SECONDSSet bridge forward delay" //usage: "\n sethello BRIDGE SECONDS Set hello time" @@ -65,11 +66,12 @@ //usage: ) // Not yet implemented: // hairpin BRIDGE IFACE on|off Hairpin on/off -// showstp BRIDGE Show stp info + #include "libbb.h" #include "common_bufsiz.h" #include +#include #include #ifndef SIOCBRADDBR @@ -85,6 +87,22 @@ # define SIOCBRDELIF BRCTL_DEL_IF #endif +#ifndef BR_STATE_DISABLED +# define BR_STATE_DISABLED 0 +#endif +#ifndef BR_STATE_LISTENING +# define BR_STATE_LISTENING 1 +#endif +#ifndef BR_STATE_LEARNING +# define BR_STATE_LEARNING 2 +#endif +#ifndef BR_STATE_FORWARDING +# define BR_STATE_FORWARDING 3 +#endif +#ifndef BR_STATE_BLOCKING +# define BR_STATE_BLOCKING 4 +#endif + #if ENABLE_FEATURE_BRCTL_FANCY static unsigned str_to_jiffies(const char *time_str) { @@ -279,6 +297,194 @@ static void show_bridge_macs(const char *name) free(fdb); } + +static int read_leaf(const char *name, const char *node, const char *leaf) +{ + char *node_path = concat_path_file(name, node); + char *leaf_path = concat_path_file(node_path, leaf); + int r = read_file(leaf_path); + free(leaf_path); + free(node_path); + return r; +} + +static void show_bridge_timer(unsigned long long timer) +{ + unsigned long long tvusec = 1ULL * timer; + unsigned int tv_sec = tvusec / 100; + unsigned int tv_usec = tvusec - (100 * tv_sec); + + printf("%4u.%.2u", tv_sec, tv_usec / 1); +} + +static const char *show_bridge_state(unsigned state) +{ + static const char *state_names[] = { + [BR_STATE_DISABLED] = "disabled", + [BR_STATE_LISTENING] = "listening", + [BR_STATE_LEARNING] = "learning", + [BR_STATE_FORWARDING] = "forwarding", + [BR_STATE_BLOCKING] = "blocking", + }; + + if (state >= 0 && state < ARRAY_SIZE(state_names)) + return state_names[state]; + + return ""; +} + +static void show_bridge_port(const char *name) +{ + read_leaf(name, "brport", "port_no"); + printf("%s (%u)\n", name, xstrtou(filedata, 0)); + + read_leaf(name, "brport", "port_id"); + printf(" port id\t\t%.4x", xstrtou(filedata, 0)); + + read_leaf(name, "brport", "state"); + printf("\t\t\tstate\t\t%15s\n", show_bridge_state(xstrtou(filedata, 0))); + + read_leaf(name, "brport", "designated_root"); + printf(" designated root\t"); + printf("%s", filedata); + + read_leaf(name, "brport", "path_cost"); + printf("\tpath cost\t\t%4u\n", xstrtou(filedata, 0)); + + read_leaf(name, "brport", "designated_bridge"); + printf(" designated bridge\t"); + printf("%s", filedata); + + read_leaf(name, "brport", "message_age_timer"); + printf("\tmessage age timer\t"); + show_bridge_timer(xstrtoull(filedata, 0)); + + read_leaf(name, "brport", "designated_port"); + printf("\n designated port\t%.4x", xstrtou(filedata, 0)); + + read_leaf(name, "brport", "forward_delay_timer"); + printf("\t\t\tforward delay timer\t"); + show_bridge_timer(xstrtoull(filedata, 0)); + + read_leaf(name, "brport", "designated_cost"); + printf("\n designated cost\t%4u", xstrtou(filedata, 0)); + + read_leaf(name, "brport", "hold_timer"); + printf("\t\t\thold timer\t\t"); + show_bridge_timer(xstrtoull(filedata, 0)); + + + printf("\n flags\t\t\t"); + read_leaf(name, "brport", "config_pending"); + if (!LONE_CHAR(filedata, '0')) + printf("CONFIG_PENDING "); + + read_leaf(name
Re: [PATCH] libbb: Converted safe_read to safe_write format
Hello, Could you please elaborate on what was lacking in the INT_MAX part? As seen in write's man page: > > On Linux, write() (and similar system calls) will transfer at most 0x7000 > (2,147,479,552) bytes, returning the number of bytes actually transferred. > (This is true on >both 32-bit and 64-bit systems.) Wouldn't it create an issue with 64 bit systems when trying to read/write large files? Thanks, Martin On Wed, 9 Oct 2019 at 07:37, Denys Vlasenko wrote: > > On Sun, Sep 15, 2019 at 6:14 PM Martin Lewis > wrote: > > > > Changed safe_read to be symmetrical to safe_write, it shall > > never return EINTR because it calls read multiple times, > > the error is considered transient. > > Applied except this part: > > > Also, as seen in gnu coreutils, handle an edge case where count is bigger > > than INT_MAX by truncating it in order to avoid bugs on various linux > > platforms. ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] telnet: added support for special telnet characters
A friendly ping for an old patch. On Mon, 8 Jul 2019 at 09:37, Martin Lewis wrote: > > Closes https://bugs.busybox.net/show_bug.cgi?id=11976 > > Adds a new flag in telnet client's command line, which > forces telnet to send IAC's instead of the > traditional special characters (backspace, newline etc..). > > This code is similar to putty's code, and is added > for compatibility with older telnet servers. > > Currently only backspace is supported, > I will add support for other characters soon. > > Signed-off-by: Martin Lewis > --- > networking/telnet.c | 40 ++-- > 1 file changed, 34 insertions(+), 6 deletions(-) > > diff --git a/networking/telnet.c b/networking/telnet.c > index fa16287..b6bebdc 100644 > --- a/networking/telnet.c > +++ b/networking/telnet.c > @@ -120,6 +120,7 @@ struct globals { > bytecharmode; > bytetelflags; > bytedo_termios; > + bytekeyboard_special_cmds; > #if ENABLE_FEATURE_TELNET_TTYPE > char*ttype; > #endif > @@ -175,6 +176,7 @@ static void con_escape(void) > full_write1_str("\r\nConsole escape. Commands are:\r\n\n" > " l go to line mode\r\n" > " c go to character mode\r\n" > + " k toggle keyboard sends telnet special > commands\r\n" > " z suspend telnet\r\n" > " e exit telnet\r\n"); > > @@ -194,6 +196,9 @@ static void con_escape(void) > goto ret; > } > break; > + case 'k': > + G.keyboard_special_cmds = !G.keyboard_special_cmds; > + break; > case 'z': > cookmode(); > kill(0, SIGTSTP); > @@ -220,15 +225,16 @@ static void handle_net_output(int len) > > while (src < end) { > byte c = *src++; > - if (c == 0x1d) { > + switch (c) { > + case 0x1d: > con_escape(); > return; > - } > - *dst = c; > - if (c == IAC) > + case IAC: > + *dst = c; > *++dst = c; /* IAC -> IAC IAC */ > - else > - if (c == '\r' || c == '\n') { > + break; > + case '\r': > + case '\n': > /* Enter key sends '\r' in raw mode and '\n' in > cooked one. > * > * See RFC 1123 3.3.1 Telnet End-of-Line Convention. > @@ -237,6 +243,28 @@ static void handle_net_output(int len) > */ > *dst = '\r'; /* Enter -> CR LF */ > *++dst = '\n'; > + break; > + > + case 0x08: /* ctrl+h */ > + case 0x7f: /* backspace */ > + if (G.keyboard_special_cmds) { > + *dst = IAC; > + *++dst = EC; > + break; > + } > + /* fall through */ > + > + case 0x03: /* ctrl+c */ > + if (G.keyboard_special_cmds) { > + *dst = IAC; > + *++dst = IP; > + break; > + } > + /* fall through */ > + > + default: > + *dst = c; > + break; > } > dst++; > } > -- > 1.9.1 > ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] libbb: Converted safe_read to safe_write format
A ping out of curiosity... On Thu, 10 Oct 2019 at 16:33, Martin Lewis wrote: > > Hello, > > Could you please elaborate on what was lacking in the INT_MAX part? > As seen in write's man page: > > > > On Linux, write() (and similar system calls) will transfer at most > > 0x7000 (2,147,479,552) bytes, returning the number of bytes actually > > transferred. (This is true on > >both 32-bit and 64-bit systems.) > > Wouldn't it create an issue with 64 bit systems when trying to > read/write large files? > > Thanks, > Martin > > > On Wed, 9 Oct 2019 at 07:37, Denys Vlasenko wrote: > > > > On Sun, Sep 15, 2019 at 6:14 PM Martin Lewis > > wrote: > > > > > > Changed safe_read to be symmetrical to safe_write, it shall > > > never return EINTR because it calls read multiple times, > > > the error is considered transient. > > > > Applied except this part: > > > > > Also, as seen in gnu coreutils, handle an edge case where count is bigger > > > than INT_MAX by truncating it in order to avoid bugs on various linux > > > platforms. ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] telnet: added support for special telnet characters
There's not much info about the exact number of devices. After a bit of searching I've found posts in various forums from 2003-2010 with similar (but not exact) backspace/new line issues. I guess some of the ancient devices don't support backspace - mostly devices with some proprietary OS or strange terminal implementation. The thing is, these IAC's should be generic as stated in telnet's RFC: > Just as the NVT data byte 68 (104 octal) should be mapped into > whatever the local code for "uppercase D" is, so the EC character > should be mapped into whatever the local "Erase Character" > function is. And from what I've seen from a couple of telnet servers (busybox excluded), they support it. On Fri, 18 Oct 2019 at 09:47, Denys Vlasenko wrote: > On Mon, Jul 8, 2019 at 4:38 PM Martin Lewis > wrote: > > Closes https://bugs.busybox.net/show_bug.cgi?id=11976 > > > > Adds a new flag in telnet client's command line, which > > forces telnet to send IAC's instead of the > > traditional special characters (backspace, newline etc..). > > > > This code is similar to putty's code, and is added > > for compatibility with older telnet servers. > > I'm not applying it yet, waiting for more user reports - > how many devices are there which don't understand backspace/DEL chars? > ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
[PATCH] nc_bloaty: Code shrink in findline, use strchr instead.
Signed-off-by: Martin Lewis --- networking/nc_bloaty.c | 21 - 1 file changed, 4 insertions(+), 17 deletions(-) diff --git a/networking/nc_bloaty.c b/networking/nc_bloaty.c index 034e03d21..9848ea794 100644 --- a/networking/nc_bloaty.c +++ b/networking/nc_bloaty.c @@ -237,24 +237,11 @@ static void arm(unsigned secs) Not distinguishing \n vs \r\n for the nonce; it just works as is... */ static unsigned findline(char *buf, unsigned siz) { - char * p; - int x; - if (!buf)/* various sanity checks... */ - return 0; - if (siz > BIGSIZ) + char *p; + if (!buf || siz > BIGSIZ) return 0; - x = siz; - for (p = buf; x > 0; x--) { - if (*p == '\n') { - x = (int) (p - buf); - x++;/* 'sokay if it points just past the end! */ -Debug("findline returning %d", x); - return x; - } - p++; - } /* for */ -Debug("findline returning whole thing: %d", siz); - return siz; + p = strchr(buf, '\n'); + return p ? (p - buf + 1) : siz; } /* findline */ /* doexec: -- 2.11.0 ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
[PATCH] nc_bloaty: Code shrink in findline, use memchr instead.
Signed-off-by: Martin Lewis --- networking/nc_bloaty.c | 21 - 1 file changed, 4 insertions(+), 17 deletions(-) diff --git a/networking/nc_bloaty.c b/networking/nc_bloaty.c index 034e03d21..190a8bcb1 100644 --- a/networking/nc_bloaty.c +++ b/networking/nc_bloaty.c @@ -237,24 +237,11 @@ static void arm(unsigned secs) Not distinguishing \n vs \r\n for the nonce; it just works as is... */ static unsigned findline(char *buf, unsigned siz) { - char * p; - int x; - if (!buf)/* various sanity checks... */ - return 0; - if (siz > BIGSIZ) + char *p; + if (!buf || siz > BIGSIZ) return 0; - x = siz; - for (p = buf; x > 0; x--) { - if (*p == '\n') { - x = (int) (p - buf); - x++;/* 'sokay if it points just past the end! */ -Debug("findline returning %d", x); - return x; - } - p++; - } /* for */ -Debug("findline returning whole thing: %d", siz); - return siz; + p = memchr(buf, '\n', siz); + return p ? (p - buf + 1) : siz; } /* findline */ /* doexec: -- 2.11.0 ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
[PATCH] xstrndup: Use strndup instead of implementing it.
Signed-off-by: Martin Lewis --- libbb/xfuncs_printf.c | 19 +-- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/libbb/xfuncs_printf.c b/libbb/xfuncs_printf.c index 93f325c62..b85dde797 100644 --- a/libbb/xfuncs_printf.c +++ b/libbb/xfuncs_printf.c @@ -93,26 +93,17 @@ char* FAST_FUNC xstrdup(const char *s) // the (possibly truncated to length n) string into it. char* FAST_FUNC xstrndup(const char *s, int n) { - int m; char *t; if (ENABLE_DEBUG && s == NULL) bb_simple_error_msg_and_die("xstrndup bug"); - /* We can just xmalloc(n+1) and strncpy into it, */ - /* but think about xstrndup("abc", 1) wastage! */ - m = n; - t = (char*) s; - while (m) { - if (!*t) break; - m--; - t++; - } - n -= m; - t = xmalloc(n + 1); - t[n] = '\0'; + t = xstrndup(s, n); - return memcpy(t, s, n); + if (t == NULL) + bb_die_memory_exhausted(); + + return t; } void* FAST_FUNC xmemdup(const void *s, int n) -- 2.11.0 ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
[PATCH] xstrndup: Use strndup instead of implementing it.
Used the same function instead of strndup by mistake... Here's the correct version: Signed-off-by: Martin Lewis --- libbb/xfuncs_printf.c | 19 +-- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/libbb/xfuncs_printf.c b/libbb/xfuncs_printf.c index 93f325c62..f1cf7aeed 100644 --- a/libbb/xfuncs_printf.c +++ b/libbb/xfuncs_printf.c @@ -93,26 +93,17 @@ char* FAST_FUNC xstrdup(const char *s) // the (possibly truncated to length n) string into it. char* FAST_FUNC xstrndup(const char *s, int n) { - int m; char *t; if (ENABLE_DEBUG && s == NULL) bb_simple_error_msg_and_die("xstrndup bug"); - /* We can just xmalloc(n+1) and strncpy into it, */ - /* but think about xstrndup("abc", 1) wastage! */ - m = n; - t = (char*) s; - while (m) { - if (!*t) break; - m--; - t++; - } - n -= m; - t = xmalloc(n + 1); - t[n] = '\0'; + t = strndup(s, n); - return memcpy(t, s, n); + if (t == NULL) + bb_die_memory_exhausted(); + + return t; } void* FAST_FUNC xmemdup(const void *s, int n) -- 2.11.0 ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
[PATCH] dhcpc: code shrink in good_hostname
Incorporated valid_domain_label into good_hostname to simplify the implementation. function old new delta static.xmalloc_optname_optval973 958 -15 -- (add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-15) Total: -15 bytes textdata bss dec hex filename 993144 169151872 1011931 f70db busybox_old 993129 169151872 1011916 f70cc busybox_unstripped Signed-off-by: Martin Lewis --- networking/udhcp/dhcpc.c | 60 +++- 1 file changed, 14 insertions(+), 46 deletions(-) diff --git a/networking/udhcp/dhcpc.c b/networking/udhcp/dhcpc.c index 5a1f8fd7a..4da80bb64 100644 --- a/networking/udhcp/dhcpc.c +++ b/networking/udhcp/dhcpc.c @@ -159,61 +159,29 @@ static int mton(uint32_t mask) } #if ENABLE_FEATURE_UDHCPC_SANITIZEOPT -/* Check if a given label represents a valid DNS label - * Return pointer to the first character after the label - * (NUL or dot) upon success, NULL otherwise. - * See RFC1035, 2.3.1 - */ +/* Check if a given name represents a valid DNS name */ +/* See RFC1035, 2.3.1 */ /* We don't need to be particularly anal. For example, allowing _, hyphen * at the end, or leading and trailing dots would be ok, since it - * can't be used for attacks. (Leading hyphen can be, if someone uses - * cmd "$hostname" + * can't be used for attacks. (Leading hyphen can be, if someone uses cmd "$hostname" * in the script: then hostname may be treated as an option) */ -static const char *valid_domain_label(const char *label) +static int good_hostname(const char *name) { unsigned char ch; - //unsigned pos = 0; - if (label[0] == '-') - return NULL; - for (;;) { - ch = *label; - if ((ch|0x20) < 'a' || (ch|0x20) > 'z') { - if (ch < '0' || ch > '9') { - if (ch == '\0' || ch == '.') - return label; - /* DNS allows only '-', but we are more permissive */ - if (ch != '-' && ch != '_') - return NULL; - } - } - label++; - //pos++; - //Do we want this? - //if (pos > 63) /* NS_MAXLABEL; labels must be 63 chars or less */ - // return NULL; - } -} + if (*name == '-') /* Can't start with '-' */ + return 0; -/* Check if a given name represents a valid DNS name */ -/* See RFC1035, 2.3.1 */ -static int good_hostname(const char *name) -{ - //const char *start = name; - - for (;;) { - name = valid_domain_label(name); - if (!name) - return 0; - if (!name[0]) - return 1; - //Do we want this? - //return ((name - start) < 1025); /* NS_MAXDNAME */ - name++; - if (*name == '\0') - return 1; // We allow trailing dot too + for (int i = 0; name[i]; i++) { + ch = name[i]; + if (!isalnum(ch)) + /* DNS allows only '-', but we are more permissive */ + if (ch != '-' && ch != '_' && ch != '.') + return 0; + // TODO: do we want to validate lengths against NS_MAXLABEL and NS_MAXDNAME? } + return 1; } #else # define good_hostname(name) 1 -- 2.11.0 ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: IPv6 - DHCPv6
Hello, I have a few notes. First of all, you should send the diff as a git patch. Not only will it make the code much easier to read, but will also make it possible to commit into the git repository. Also you mentioned that the server still doesn't respond as expected, perhaps you could address that too? Martin On Tue, 5 May 2020 at 12:29, Uwe Glaeser wrote: > Hi all, > > > > I try to get a IPv6 address via DHCP from a “Windows 2012 R2 Server with > DHCPv6 Server”. > > With actual busybox 1.31.1 and enabled IPv6 and udhcpc6 feature I try to > make contact to the server. > > My deivce is an embedded Linux 2.6 > > What is working perfectly well is busybox ping6 and I get an IPv6 address > when connecting my PC(Win 10) to the server. > > I watched the network with Wireshark and found out, that the ping6 and the > dhcp-solicit from the PC are transmitted as ipv6-multicast. > > But the udhcpc6 solicit message is sent as IPv4 broadcast (with > FF:FF:FF:FF:FF:FF as destination in the EthernetII frame) which is not > correct I think. > > I have made some changes in the sources that way: > > > > I defined the field in common.h/.c for the IPv6 multicast to a DHCPv6 > (general address is FF02::1:2) > > > > const uint8_t MAC_DHCPBCAST_ADDR[6] ALIGN2= { > > 0x33, 0x33, 0x00, 0x01, 0x00, 0x02 > > }; > > > > and used it in d6_dhcp.c in the call of: > > > > static int d6_mcast_from_client_data_ifindex(struct d6_packet *packet, > uint8_t *end) > > { > > /* FF02::1:2 is "All_DHCP_Relay_Agents_and_Servers" > address */ > > static const uint8_t FF02__1_2[16] = { > >0xFF, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, > 0x00, > >0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, > 0x02, > > }; > > > > return d6_send_raw_packet( packet, (end - (uint8_t*) > packet), > > >/*src*/ > &client6_data.ll_ip6, CLIENT_PORT6, > > > /*dst*/ (struct in6_addr*)FF02__1_2, SERVER_PORT6, MAC_DHCPBCAST_ADDR, > > > client_data.ifindex > > ); > > } > > > > That works better now and I can see nearly the same contents in Wireshark > as in the PC-dhcpv6 request. Ther Server still does not answer but that may > have other reasons… > > I’m not sure if there are other parts where it would make sense to change > the header…Please have a look! > > > > Best regards! > > > > Uwe Glaeser > dormakaba EAD GmbH > ___ > busybox mailing list > busybox@busybox.net > http://lists.busybox.net/mailman/listinfo/busybox > ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: Busybox hush shell does not read /etc/profile
Hi, /etc/profile is read only during interactive login shell mode (hush's -l flag). I think that a tty must be attached to the shell for this to work, not sure if that's the case with inittab. Martin On Wed, 26 Feb 2020 at 18:37, Carlos Eduardo de Paula wrote: > I've been struggling to understand on how to make Hush shell read my > /etc/profile. > > I've configured my inittab with: > > ::respawn:/bin/cttyhack /bin/login -f root > > And also tried with the leading dash: > > ttySIF0::respawn:-/bin/sh > > In both cases, the profile is not read. > > My code is in > https://github.com/carlosedp/riscv64-nommu-buildroot/tree/k210/board/kendryte/k210 > > What am I missing? > > Thanks for any help! > > > -- > > Carlos Eduardo de Paula > m...@carlosedp.com > http://carlosedp.com > http://twitter.com/carlosedp > Linkedin > > ___ > busybox mailing list > busybox@busybox.net > http://lists.busybox.net/mailman/listinfo/busybox > ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: How to redirect the output of lbb_main function to variable on c program
Hello, Sounds like you need output redirection. You need to create a pipe, dup2 it over stdout, call lbb_main, and read the output from the pipe. Similar question: https://stackoverflow.com/questions/17071702/c-language-read-from-stdout Martin On 22/05/2020, Tiago Araujo da costa wrote: > I build libbusybox.so.1.31.1 and put on /usr/local/lib. In addition, I > created a symbolic link /usr/local/lib/libbusybox.soI have a file, the > content of which is: > $ cat ogt.cextern int lbb_main(char **argv); > > int main() > { > char* strarray[] = {"ifconfig",0}; > lbb_main(strarray); > > return 1; > } > I compile the above file with the following command: > $ gcc -o ogt ogt.c -lbusybox > And run: > $ ./ogteth0 Link encap:Ethernet HWaddr 07:1F:EA:C9:3B:4C > inet addr:172.16.51.62 Bcast:172.16.51.255 Mask:255.255.255.0 > UP BROADCAST RUNNING MULTICAST MTU:1500 Metric:1 > RX packets:7989695 errors:0 dropped:258 overruns:0 frame:0 > TX packets:4696710 errors:0 dropped:0 overruns:0 carrier:0 > collisions:0 txqueuelen:1000 > RX bytes:2659885363 (2.4 GiB) TX bytes:7111442553 (6.6 GiB) > > lo Link encap:Local Loopback > inet addr:127.0.0.1 Mask:255.0.0.0 > UP LOOPBACK RUNNING MTU:65536 Metric:1 > RX packets:2654 errors:0 dropped:0 overruns:0 frame:0 > TX packets:2654 errors:0 dropped:0 overruns:0 carrier:0 > collisions:0 txqueuelen:1000 > RX bytes:225192 (219.9 KiB) TX bytes:225192 (219.9 KiB) > How do I redirect the output of command "lbb_main (strarray);" for a > variable, inside the ogt.c program? > ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
[PATCH] A friendly ping for code shrinks
Hello, Just a friendly ping for some old binary/code shrinking patches. dhcpc: http://lists.busybox.net/pipermail/busybox/2020-May/087942.html xfuncs_printf.c: http://lists.busybox.net/pipermail/busybox/2020-March/087822.html function old new delta xstrndup 64 24 -40 -- (add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-40) Total: -40 bytes textdata bss dec hex filename 993204 169151872 1011991 f7117 busybox_old 993238 169231872 1012033 f7141 busybox_unstripped Martin ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
[PATCH] domain_codec: optimize call to dname_enc
The only call to dname_enc is with cstr = NULL, so most of dname_enc's logic is not used. Therefore, we can directly call convert_dname and shrink the binary size. function old new delta convert_dname - 143+143 attach_option463 493 +30 dname_enc445 --445 -- (add/remove: 1/1 grow/shrink: 1/0 up/down: 173/-445) Total: -272 bytes textdata bss dec hex filename 993252 169231872 1012047 f714f busybox_old 992980 169231872 1011775 f703f busybox_unstripped Signed-off-by: Martin Lewis --- networking/udhcp/common.c | 6 +- networking/udhcp/common.h | 1 + networking/udhcp/domain_codec.c | 2 +- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/networking/udhcp/common.c b/networking/udhcp/common.c index 9ec752dfc..f609bcfdb 100644 --- a/networking/udhcp/common.c +++ b/networking/udhcp/common.c @@ -431,7 +431,11 @@ static NOINLINE void attach_option( #if ENABLE_FEATURE_UDHCP_RFC3397 if ((optflag->flags & OPTION_TYPE_MASK) == OPTION_DNS_STRING) { /* reuse buffer and length for RFC1035-formatted string */ - allocated = buffer = (char *)dname_enc(NULL, 0, buffer, &length); + allocated = buffer = (char *)convert_dname(buffer); + if (buffer == NULL) + length = 0; + else + length = strlen(buffer) + 1; /* including NUL */ } #endif diff --git a/networking/udhcp/common.h b/networking/udhcp/common.h index 60255eefa..7aeeb5152 100644 --- a/networking/udhcp/common.h +++ b/networking/udhcp/common.h @@ -219,6 +219,7 @@ void udhcp_add_simple_option(struct dhcp_packet *packet, uint8_t code, uint32_t #if ENABLE_FEATURE_UDHCP_RFC3397 || ENABLE_FEATURE_UDHCPC6_RFC3646 || ENABLE_FEATURE_UDHCPC6_RFC4704 char *dname_dec(const uint8_t *cstr, int clen, const char *pre) FAST_FUNC; uint8_t *dname_enc(const uint8_t *cstr, int clen, const char *src, int *retlen) FAST_FUNC; +uint8_t *convert_dname(const char *src); #endif struct option_set *udhcp_find_option(struct option_set *opt_list, uint8_t code) FAST_FUNC; diff --git a/networking/udhcp/domain_codec.c b/networking/udhcp/domain_codec.c index b7a3a5353..b5df48d87 100644 --- a/networking/udhcp/domain_codec.c +++ b/networking/udhcp/domain_codec.c @@ -113,7 +113,7 @@ char* FAST_FUNC dname_dec(const uint8_t *cstr, int clen, const char *pre) * RFC1035 encoding "\003foo\004blah\003com\000". Return allocated string, or * NULL if an error occurs. */ -static uint8_t *convert_dname(const char *src) +uint8_t *convert_dname(const char *src) { uint8_t c, *res, *lenptr, *dst; int len; -- 2.11.0 ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
[PATCH] dhcpc: refactor xmalloc_optname_optval to shrink binary size
function old new delta len_of_option_as_string 14 13 -1 dhcp_option_lengths 14 13 -1 udhcp_str2optset 717 715 -2 .rodata 159869 159867 -2 static.xmalloc_optname_optval959 828-131 -- (add/remove: 0/0 grow/shrink: 0/5 up/down: 0/-137) Total: -137 bytes textdata bss dec hex filename 993252 169231872 1012047 f714f busybox_old 993117 169231872 1011912 f70c8 busybox_unstripped Signed-off-by: Martin Lewis --- networking/udhcp/common.h | 2 +- networking/udhcp/dhcpc.c | 47 --- 2 files changed, 17 insertions(+), 32 deletions(-) diff --git a/networking/udhcp/common.h b/networking/udhcp/common.h index 60255eefa..0640c1241 100644 --- a/networking/udhcp/common.h +++ b/networking/udhcp/common.h @@ -78,7 +78,7 @@ struct BUG_bad_sizeof_struct_ip_udp_dhcp_packet { /*** Options ***/ enum { - OPTION_IP = 1, + OPTION_IP = 0, OPTION_IP_PAIR, OPTION_STRING, /* Opts of STRING_HOST type will be sanitized before they are passed diff --git a/networking/udhcp/dhcpc.c b/networking/udhcp/dhcpc.c index 6422181da..5cff53d60 100644 --- a/networking/udhcp/dhcpc.c +++ b/networking/udhcp/dhcpc.c @@ -208,9 +208,8 @@ static NOINLINE char *xmalloc_optname_optval(uint8_t *option, const struct dhcp_ case OPTION_IP: case OPTION_IP_PAIR: dest += sprint_nip(dest, "", option); - if (type == OPTION_IP) - break; - dest += sprint_nip(dest, "/", option + 4); + if (type == OPTION_IP_PAIR) + dest += sprint_nip(dest, "/", option + 4); break; // case OPTION_BOOLEAN: // dest += sprintf(dest, *option ? "yes" : "no"); @@ -262,7 +261,6 @@ static NOINLINE char *xmalloc_optname_optval(uint8_t *option, const struct dhcp_ while (len >= 1 + 4) { /* mask + 0-byte ip + router */ uint32_t nip; - uint8_t *p; unsigned mask; int bytes; @@ -272,12 +270,12 @@ static NOINLINE char *xmalloc_optname_optval(uint8_t *option, const struct dhcp_ len--; nip = 0; - p = (void*) &nip; bytes = (mask + 7) / 8; /* 0 -> 0, 1..8 -> 1, 9..16 -> 2 etc */ - while (--bytes >= 0) { - *p++ = *option++; - len--; - } + + memcpy(&nip, option, bytes); + len -= bytes; + option += bytes; + if (len < 4) break; @@ -326,17 +324,11 @@ static NOINLINE char *xmalloc_optname_optval(uint8_t *option, const struct dhcp_ /* 6rdPrefix */ dest += sprint_nip6(dest, /* "", */ option); option += 16; - len -= 1 + 1 + 16 + 4; - /* "+ 4" above corresponds to the length of IPv4 addr -* we consume in the loop below */ - while (1) { - /* 6rdBRIPv4Address(es) */ - dest += sprint_nip(dest, " ", option); - option += 4; - len -= 4; /* do we have yet another 4+ bytes? */ - if (len < 0) - break; /* no */ - } + len -= 1 + 1 + 16; + *dest++ = ' '; + type = OPTION_IP; + optlen = 4; + continue; } return ret; @@ -364,17 +356,10 @@ static NOINLINE char *xmalloc_optname_optval(uint8_t *option, const struct dhcp_ free(ret); return dest; } -
Re: [PATCH] domain_codec: optimize call to dname_enc
Hi, Yes, looks good :) Martin On Wed, 10 Jun 2020 at 10:21, Denys Vlasenko wrote: > Thanks. I addressed this a bit differently. Looks good now? > > On Tue, Jun 9, 2020 at 4:55 PM Martin Lewis > wrote: > > > > The only call to dname_enc is with cstr = NULL, so most of dname_enc's > logic is not used. > > Therefore, we can directly call convert_dname and shrink the binary size. > > > > function old new delta > > convert_dname - 143+143 > > attach_option463 493 +30 > > dname_enc445 --445 > > > -- > > (add/remove: 1/1 grow/shrink: 1/0 up/down: 173/-445) Total: -272 > > bytes > >textdata bss dec hex filename > > 993252 169231872 1012047 f714f busybox_old > > 992980 169231872 1011775 f703f busybox_unstripped > > > > Signed-off-by: Martin Lewis > > --- > > networking/udhcp/common.c | 6 +- > > networking/udhcp/common.h | 1 + > > networking/udhcp/domain_codec.c | 2 +- > > 3 files changed, 7 insertions(+), 2 deletions(-) > > > > diff --git a/networking/udhcp/common.c b/networking/udhcp/common.c > > index 9ec752dfc..f609bcfdb 100644 > > --- a/networking/udhcp/common.c > > +++ b/networking/udhcp/common.c > > @@ -431,7 +431,11 @@ static NOINLINE void attach_option( > > #if ENABLE_FEATURE_UDHCP_RFC3397 > > if ((optflag->flags & OPTION_TYPE_MASK) == OPTION_DNS_STRING) { > > /* reuse buffer and length for RFC1035-formatted string > */ > > - allocated = buffer = (char *)dname_enc(NULL, 0, buffer, > &length); > > + allocated = buffer = (char *)convert_dname(buffer); > > + if (buffer == NULL) > > + length = 0; > > + else > > + length = strlen(buffer) + 1; /* including NUL */ > > } > > #endif > > > > diff --git a/networking/udhcp/common.h b/networking/udhcp/common.h > > index 60255eefa..7aeeb5152 100644 > > --- a/networking/udhcp/common.h > > +++ b/networking/udhcp/common.h > > @@ -219,6 +219,7 @@ void udhcp_add_simple_option(struct dhcp_packet > *packet, uint8_t code, uint32_t > > #if ENABLE_FEATURE_UDHCP_RFC3397 || ENABLE_FEATURE_UDHCPC6_RFC3646 || > ENABLE_FEATURE_UDHCPC6_RFC4704 > > char *dname_dec(const uint8_t *cstr, int clen, const char *pre) > FAST_FUNC; > > uint8_t *dname_enc(const uint8_t *cstr, int clen, const char *src, int > *retlen) FAST_FUNC; > > +uint8_t *convert_dname(const char *src); > > #endif > > struct option_set *udhcp_find_option(struct option_set *opt_list, > uint8_t code) FAST_FUNC; > > > > diff --git a/networking/udhcp/domain_codec.c > b/networking/udhcp/domain_codec.c > > index b7a3a5353..b5df48d87 100644 > > --- a/networking/udhcp/domain_codec.c > > +++ b/networking/udhcp/domain_codec.c > > @@ -113,7 +113,7 @@ char* FAST_FUNC dname_dec(const uint8_t *cstr, int > clen, const char *pre) > > * RFC1035 encoding "\003foo\004blah\003com\000". Return allocated > string, or > > * NULL if an error occurs. > > */ > > -static uint8_t *convert_dname(const char *src) > > +uint8_t *convert_dname(const char *src) > > { > > uint8_t c, *res, *lenptr, *dst; > > int len; > > -- > > 2.11.0 > > > > ___ > > busybox mailing list > > busybox@busybox.net > > http://lists.busybox.net/mailman/listinfo/busybox > ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
[PATCH 1/4] procps: code shrink
function old new delta skip_whitespace_if_prefixed_with - 37 +37 procps_read_smaps954 841-113 -- (add/remove: 1/0 grow/shrink: 0/1 up/down: 37/-113) Total: -76 bytes textdata bss dec hex filename 981418 169151872 1000205 f430d busybox_old 981342 169151872 1000129 f42c1 busybox_unstripped Signed-off-by: Martin Lewis --- libbb/procps.c | 12 ++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/libbb/procps.c b/libbb/procps.c index af3ad86ff..c7ce45977 100644 --- a/libbb/procps.c +++ b/libbb/procps.c @@ -177,6 +177,15 @@ static char *skip_fields(char *str, int count) } #endif +static int skip_whitespace_if_prefixed_with(char *buf, const char *prefix, char **tp) { + *tp = is_prefixed_with(buf, prefix); + if (*tp) { + *tp = skip_whitespace(*tp); + return TRUE; + } + return FALSE; +} + #if ENABLE_FEATURE_TOPMEM || ENABLE_PMAP int FAST_FUNC procps_read_smaps(pid_t pid, struct smaprec *total, void (*cb)(struct smaprec *, void *), void *data) @@ -207,8 +216,7 @@ int FAST_FUNC procps_read_smaps(pid_t pid, struct smaprec *total, char *tp, *p; #define SCAN(S, X) \ - if ((tp = is_prefixed_with(buf, S)) != NULL) { \ - tp = skip_whitespace(tp);\ + if (skip_whitespace_if_prefixed_with(buf, S, &tp)) { \ total->X += currec.X = fast_strtoul_10(&tp); \ continue;\ } -- 2.11.0 ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
[PATCH 4/4] last_char_is: code shrink
function old new delta last_char_is 53 30 -23 -- (add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-23) Total: -23 bytes textdata bss dec hex filename 981322 169151872 1000109 f42ad busybox_old 981299 169151872 186 f4296 busybox_unstripped Signed-off-by: Martin Lewis --- libbb/last_char_is.c | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/libbb/last_char_is.c b/libbb/last_char_is.c index 66f2e3635..1fff08f9f 100644 --- a/libbb/last_char_is.c +++ b/libbb/last_char_is.c @@ -13,11 +13,10 @@ */ char* FAST_FUNC last_char_is(const char *s, int c) { - if (s && *s) { - size_t sz = strlen(s) - 1; - s += sz; - if ( (unsigned char)*s == c) - return (char*)s; + if (s) { + char *index = strrchr(s, c); + if (index && *(index + 1) == '\0') + return index; } return NULL; } -- 2.11.0 ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
[PATCH 3/4] lsscsi: code shrink and refactor
Remove the use of strou because scsi device types are signed (as seen in the kernel's code). Improve the representation of SCSI_DEVICE_LIST. function old new delta .rodata 159915 159920 +5 lsscsi_main 364 349 -15 -- (add/remove: 0/0 grow/shrink: 1/1 up/down: 5/-15) Total: -10 bytes textdata bss dec hex filename 981332 169151872 1000119 f42b7 busybox_old 981322 169151872 1000109 f42ad busybox_unstripped Signed-off-by: Martin Lewis --- miscutils/lsscsi.c | 50 ++ 1 file changed, 34 insertions(+), 16 deletions(-) diff --git a/miscutils/lsscsi.c b/miscutils/lsscsi.c index f737d33d9..e29bedd77 100644 --- a/miscutils/lsscsi.c +++ b/miscutils/lsscsi.c @@ -25,6 +25,28 @@ #include "libbb.h" +#define SCSI_DEVICE_TYPE(type, name) type name "\0" +#define SCSI_DEVICE_TYPE_LIST \ + SCSI_DEVICE_TYPE("\x00", "disk") \ + SCSI_DEVICE_TYPE("\x01", "tape") \ + SCSI_DEVICE_TYPE("\x02", "printer") \ + SCSI_DEVICE_TYPE("\x03", "process") \ + SCSI_DEVICE_TYPE("\x04", "worm") \ + SCSI_DEVICE_TYPE("\x06", "scanner") \ + SCSI_DEVICE_TYPE("\x07", "optical") \ + SCSI_DEVICE_TYPE("\x08", "mediumx") \ + SCSI_DEVICE_TYPE("\x09", "comms") \ + SCSI_DEVICE_TYPE("\x0c", "storage") \ + SCSI_DEVICE_TYPE("\x0d", "enclosu") \ + SCSI_DEVICE_TYPE("\x0e", "sim dsk") \ + SCSI_DEVICE_TYPE("\x0f", "opti rd") \ + SCSI_DEVICE_TYPE("\x10", "bridge") \ + SCSI_DEVICE_TYPE("\x11", "osd") \ + SCSI_DEVICE_TYPE("\x12", "adi") \ + SCSI_DEVICE_TYPE("\x1e", "wlun") \ + SCSI_DEVICE_TYPE("\x1f", "no dev") + + static const char scsi_dir[] ALIGN1 = "/sys/bus/scsi/devices"; static char *get_line(const char *filename, char *buf, unsigned *bufsize_p) @@ -59,6 +81,13 @@ int lsscsi_main(int argc UNUSED_PARAM, char **argv UNUSED_PARAM) dir = xopendir("."); while ((de = readdir(dir)) != NULL) { + /* +* From /drivers/scsi/scsi_sysfs.c in the kernel: +* sdev_rd_attr (type, "%d\n"); +* sdev_rd_attr (vendor, "%.8s\n"); +* sdev_rd_attr (model, "%.16s\n"); +* sdev_rd_attr (rev, "%.4s\n"); +*/ char buf[256]; char *ptr; unsigned bufsize; @@ -67,7 +96,7 @@ int lsscsi_main(int argc UNUSED_PARAM, char **argv UNUSED_PARAM) const char *type_name; const char *model; const char *rev; - unsigned type; + int type; if (!isdigit(de->d_name[0])) continue; @@ -88,23 +117,12 @@ int lsscsi_main(int argc UNUSED_PARAM, char **argv UNUSED_PARAM) printf("[%s]\t", de->d_name); -#define scsi_device_types \ - "disk\0""tape\0""printer\0" "process\0" \ - "worm\0""\0""scanner\0" "optical\0" \ - "mediumx\0" "comms\0" "\0""\0"\ - "storage\0" "enclosu\0" "sim dsk\0" "opti rd\0" \ - "bridge\0" "osd\0" "adi\0" "\0"\ - "\0""\0""\0""\0"\ - "\0""\0""\0""\0"\ - "\0""\0""wlun\0""no dev" - type = bb_strtou(type_str, NULL, 10); - if (errno -|| type >= 0x20 -|| (type_name = nth_string(scsi_device_types, type))[0] == '\0' - ) { + type = atoi(type_str); /* type_str is "%d\n" so atoi is sufficient */ + if (type >= 0x20 || + (type_name = memchr(SCSI_DEVICE_TYPE_LIST, type, sizeof(SCSI_DEVICE_TYPE_LIST))) == NULL) { printf("(%s)\t", type_str); } else { - printf("%s\t", type_name); + printf("%s\t", type_name + 1); } printf("%s\t""%s\t""%s\n", -- 2.11.0 ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
[PATCH 2/4] compare_string_array: code shrink
Code shrink and prevention of possible out of bounds access. function old new delta nth_string36 26 -10 -- (add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-10) Total: -10 bytes textdata bss dec hex filename 981342 169151872 1000129 f42c1 busybox_old 981332 169151872 1000119 f42b7 busybox_unstripped Signed-off-by: Martin Lewis --- libbb/compare_string_array.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/libbb/compare_string_array.c b/libbb/compare_string_array.c index 01a9df0e2..a06e57d3d 100644 --- a/libbb/compare_string_array.c +++ b/libbb/compare_string_array.c @@ -117,8 +117,11 @@ int FAST_FUNC index_in_substrings(const char *strings, const char *key) const char* FAST_FUNC nth_string(const char *strings, int n) { while (n) { - n--; - strings += strlen(strings) + 1; + if (*strings++ == '\0') { + if (*strings == '\0') /* reached end of strings */ + break; + n--; + } } return strings; } -- 2.11.0 ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
[PATCH 2/2] udhcpc: fixed a TODO in fill_envp using option scanner
fill_envp now iterates over the packet only once instead of a few hundred times using the new option scanner. Signed-off-by: Martin Lewis --- networking/udhcp/dhcpc.c | 201 --- 1 file changed, 87 insertions(+), 114 deletions(-) diff --git a/networking/udhcp/dhcpc.c b/networking/udhcp/dhcpc.c index 102178a4f..2669428e1 100644 --- a/networking/udhcp/dhcpc.c +++ b/networking/udhcp/dhcpc.c @@ -386,59 +386,81 @@ static NOINLINE char *xmalloc_optname_optval(uint8_t *option, const struct dhcp_ return ret; } +static void putenvp(llist_t **envp, char *new_opt) +{ + putenv(new_opt); + llist_add_to(envp, new_opt); +} + +static int is_unknown_opt(uint8_t code, const struct dhcp_optflag **dh, const char **opt_name) +{ +*opt_name = ""; + +/* Find the option: + * dhcp_optflags is sorted so we stop searching when dh->code >= code, which is faster + * than iterating over the entire array. + * Options which don't have a match in dhcp_option_strings[], e.g DHCP_REQUESTED_IP, + * are located after the sorted array, so these entries will never be reached + * and they'll count as unknown options. + */ +for (*dh = dhcp_optflags; (*dh)->code && (*dh)->code < code; (*dh)++); + +if ((*dh)->code == code) +*opt_name = nth_string(dhcp_option_strings, (*dh - dhcp_optflags)); + +return (*dh)->code > code; +} + /* put all the parameters into the environment */ -static char **fill_envp(struct dhcp_packet *packet) +static llist_t *fill_envp(struct dhcp_packet *packet) { - int envc; - int i; - char **envp, **curr; - const char *opt_name; - uint8_t *temp; - uint8_t overload = 0; - -#define BITMAP unsigned -#define BBITS (sizeof(BITMAP) * 8) -#define BMASK(i) (1 << (i & (sizeof(BITMAP) * 8 - 1))) -#define FOUND_OPTS(i) (found_opts[(unsigned)i / BBITS]) - BITMAP found_opts[256 / BBITS]; - - memset(found_opts, 0, sizeof(found_opts)); - - /* We need 7 elements for: -* "interface=IFACE" -* "ip=N.N.N.N" from packet->yiaddr -* "giaddr=IP" from packet->gateway_nip (unless 0) -* "siaddr=IP" from packet->siaddr_nip (unless 0) -* "boot_file=FILE" from packet->file (unless overloaded) -* "sname=SERVER_HOSTNAME" from packet->sname (unless overloaded) -* terminating NULL -*/ - envc = 7; - /* +1 element for each option, +2 for subnet option: */ - if (packet) { - /* note: do not search for "pad" (0) and "end" (255) options */ -//TODO: change logic to scan packet _once_ - for (i = 1; i < 255; i++) { - temp = udhcp_get_option(packet, i); - if (temp) { - if (i == DHCP_OPTION_OVERLOAD) - overload |= *temp; - else if (i == DHCP_SUBNET) - envc++; /* for $mask */ - envc++; - /*if (i != DHCP_MESSAGE_TYPE)*/ - FOUND_OPTS(i) |= BMASK(i); - } - } - } - curr = envp = xzalloc(sizeof(envp[0]) * envc); + char *new_opt; + uint8_t *optptr; + const struct dhcp_optflag *dh; + struct dhcp_scan_state scan_state; + const char *opt_name = ""; + llist_t *envp = NULL; - *curr = xasprintf("interface=%s", client_data.interface); - putenv(*curr++); + new_opt = xasprintf("interface=%s", client_data.interface); + putenvp(&envp, new_opt); if (!packet) return envp; + init_scan_state(packet, &scan_state); + +/* Iterate over the packet options. + * Handle each option based on whether it's an unknown / known option. + * There may be (although unlikely) duplicate options. For now, only the last + * appearing option will be stored in the environment, and all duplicates + * are freed properly. + * Long options may be implemented in the future (see RFC 3396) if needed. + */ + while ((optptr = udhcp_scan_options(packet, &scan_state)) != NULL) { + uint8_t code = optptr[OPT_CODE]; + uint8_t len = optptr[OPT_LEN]; + uint8_t *data = optptr + OPT_DATA; + + if (is_unknown_opt(code, &dh, &opt_name)) { + unsigned ofs; + + new_opt = xmalloc(sizeof("optNNN=") + 1 + len*2); + ofs = sprintf(new_opt, "opt%u=", code); + *bin2hex(new_opt + ofs, (char *)data, len) = '\0'; +
[PATCH 1/2] udhcp: add option scanner
Added an option scanner to udhcp to enable iteration over packet options. Signed-off-by: Martin Lewis --- networking/udhcp/common.c | 96 ++- networking/udhcp/common.h | 8 2 files changed, 62 insertions(+), 42 deletions(-) diff --git a/networking/udhcp/common.c b/networking/udhcp/common.c index 16bf69707..20d843bab 100644 --- a/networking/udhcp/common.c +++ b/networking/udhcp/common.c @@ -15,7 +15,7 @@ const uint8_t MAC_BCAST_ADDR[6] ALIGN2 = { }; #if ENABLE_UDHCPC || ENABLE_UDHCPD -/* Supported options are easily added here. +/* Supported options are easily added here, they need to be sorted. * See RFC2132 for more options. * OPTION_REQ: these options are requested by udhcpc (unless -o). */ @@ -222,79 +222,91 @@ unsigned FAST_FUNC udhcp_option_idx(const char *name, const char *option_strings } } -/* Get an option with bounds checking (warning, result is not aligned) */ -uint8_t* FAST_FUNC udhcp_get_option(struct dhcp_packet *packet, int code) +/* Initialize state to be used between subsequent udhcp_scan_options calls */ +void FAST_FUNC init_scan_state(struct dhcp_packet *packet, struct dhcp_scan_state *scan_state) +{ + scan_state->overload = 0; + scan_state->rem = sizeof(packet->options); + scan_state->optionptr = packet->options; +} + +/* Iterate over packet's options, each call returning the next option. + * scan_state needs to be initialized with init_scan_state beforehand. + * Warning, result is not aligned. */ +uint8_t* FAST_FUNC udhcp_scan_options(struct dhcp_packet *packet, struct dhcp_scan_state *scan_state) { - uint8_t *optionptr; int len; - int rem; - int overload = 0; enum { FILE_FIELD101 = FILE_FIELD * 0x101, SNAME_FIELD101 = SNAME_FIELD * 0x101, }; /* option bytes: [code][len][data1][data2]..[dataLEN] */ - optionptr = packet->options; - rem = sizeof(packet->options); while (1) { - if (rem <= 0) { + if (scan_state->rem <= 0) { complain: bb_simple_error_msg("bad packet, malformed option field"); return NULL; } /* DHCP_PADDING and DHCP_END have no [len] byte */ - if (optionptr[OPT_CODE] == DHCP_PADDING) { - rem--; - optionptr++; + if (scan_state->optionptr[OPT_CODE] == DHCP_PADDING) { + scan_state->rem--; + scan_state->optionptr++; continue; } - if (optionptr[OPT_CODE] == DHCP_END) { - if ((overload & FILE_FIELD101) == FILE_FIELD) { + if (scan_state->optionptr[OPT_CODE] == DHCP_END) { + if ((scan_state->overload & FILE_FIELD101) == FILE_FIELD) { /* can use packet->file, and didn't look at it yet */ - overload |= FILE_FIELD101; /* "we looked at it" */ - optionptr = packet->file; - rem = sizeof(packet->file); + scan_state->overload |= FILE_FIELD101; /* "we looked at it" */ + scan_state->optionptr = packet->file; + scan_state->rem = sizeof(packet->file); continue; } - if ((overload & SNAME_FIELD101) == SNAME_FIELD) { + if ((scan_state->overload & SNAME_FIELD101) == SNAME_FIELD) { /* can use packet->sname, and didn't look at it yet */ - overload |= SNAME_FIELD101; /* "we looked at it" */ - optionptr = packet->sname; - rem = sizeof(packet->sname); + scan_state->overload |= SNAME_FIELD101; /* "we looked at it" */ + scan_state->optionptr = packet->sname; + scan_state->rem = sizeof(packet->sname); continue; } break; } - if (rem <= OPT_LEN) + if (scan_state->rem <= OPT_LEN) goto complain; /* complain and return NULL */ - len = 2 + optionptr[OPT_LEN]; - rem -= len; - if (rem < 0) + len = 2 + scan_state->optionptr[OPT_LEN]; + scan_state->rem -= len; + /* So far no valid option with length 0 known. */ +
[PATCH 0/2] udhcpc: optimized fill_envp (fixes TODO)
Up until now, udhcpc iterated over the received packet a few hundred times (!). This patch series resolves the relevant TODO - iterating over the packet just once, while also reducing binary size. Tested against: * VMware's dhcp server * udhcpd * ISC's dhcpd function old new delta udhcp_scan_options - 203+203 putenvp- 39 +39 init_scan_state- 24 +24 udhcp_get_option 200 107 -93 udhcp_run_script 963 722-241 -- (add/remove: 3/0 grow/shrink: 0/2 up/down: 266/-334) Total: -68 bytes textdata bss dec hex filename 981424 169151872 1000211 f4313 busybox_old 981356 169151872 1000143 f42cf busybox_unstripped ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH 4/4] last_char_is: code shrink
Hi, I'm not sure what I'm missing, from a quick glance at strrchr (glibc's) it seems that it scans the string only once? Thank you, Martin On Tue, 30 Jun 2020 at 01:34, Denys Vlasenko wrote: > This scans the string twice, unnecessarily. Let's not do that. > > On Thu, Jun 11, 2020 at 3:45 PM Martin Lewis > wrote: > > > > function old new delta > > last_char_is 53 30 -23 > > > -- > > (add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-23) Total: -23 > bytes > >textdata bss dec hex filename > > 981322 169151872 1000109 f42ad busybox_old > > 981299 169151872 186 f4296 busybox_unstripped > > > > Signed-off-by: Martin Lewis > > --- > > libbb/last_char_is.c | 9 - > > 1 file changed, 4 insertions(+), 5 deletions(-) > > > > diff --git a/libbb/last_char_is.c b/libbb/last_char_is.c > > index 66f2e3635..1fff08f9f 100644 > > --- a/libbb/last_char_is.c > > +++ b/libbb/last_char_is.c > > @@ -13,11 +13,10 @@ > > */ > > char* FAST_FUNC last_char_is(const char *s, int c) > > { > > - if (s && *s) { > > - size_t sz = strlen(s) - 1; > > - s += sz; > > - if ( (unsigned char)*s == c) > > - return (char*)s; > > + if (s) { > > + char *index = strrchr(s, c); > > + if (index && *(index + 1) == '\0') > > + return index; > > } > > return NULL; > > } > > -- > > 2.11.0 > > > > ___ > > busybox mailing list > > busybox@busybox.net > > http://lists.busybox.net/mailman/listinfo/busybox > ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH 4/4] last_char_is: code shrink
Notice that there's a "-" before the line with strlen. Here's the final implementation to simplify reading: char* FAST_FUNC last_char_is(const char *s, int c) { if (s) { char *index = strrchr(s, c); if (index && *(index + 1) == '\0') return index; } return NULL; } On Tue, 30 Jun 2020 at 10:59, Jody Bruchon wrote: > strlen scans the string; strrchr also scans the string. > > On 2020-07-01 10:23, Martin Lewis wrote: > > Hi, > > > > I'm not sure what I'm missing, from a quick glance at strrchr > > (glibc's) it seems that it scans the string only once? > > > > Thank you, > > Martin > > > > On Tue, 30 Jun 2020 at 01:34, Denys Vlasenko > <mailto:vda.li...@googlemail.com>> wrote: > > > > This scans the string twice, unnecessarily. Let's not do that. > > > > On Thu, Jun 11, 2020 at 3:45 PM Martin Lewis > > mailto:martin.lewis@gmail.com>> > > wrote: > > > > > > function oldnew > > delta > > > last_char_is 53 30 > >-23 > > > > > > > -- > > > (add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-23) Total: > > -23 bytes > > >textdata bss dec hex filename > > > 981322 169151872 1000109 f42ad busybox_old > > > 981299 169151872 186 f4296 busybox_unstripped > > > > > > Signed-off-by: Martin Lewis > <mailto:martin.lewis@gmail.com>> > > > --- > > > libbb/last_char_is.c | 9 - > > > 1 file changed, 4 insertions(+), 5 deletions(-) > > > > > > diff --git a/libbb/last_char_is.c b/libbb/last_char_is.c > > > index 66f2e3635..1fff08f9f 100644 > > > --- a/libbb/last_char_is.c > > > +++ b/libbb/last_char_is.c > > > @@ -13,11 +13,10 @@ > > > */ > > > char* FAST_FUNC last_char_is(const char *s, int c) > > > { > > > - if (s && *s) { > > > - size_t sz = strlen(s) - 1; > > > - s += sz; > > > - if ( (unsigned char)*s == c) > > > - return (char*)s; > > > + if (s) { > > > + char *index = strrchr(s, c); > > > + if (index && *(index + 1) == '\0') > > > + return index; > > > } > > > return NULL; > > > } > > > -- > > > 2.11.0 > > > > > > ___ > > > busybox mailing list > > > busybox@busybox.net <mailto:busybox@busybox.net> > > > http://lists.busybox.net/mailman/listinfo/busybox > > > > > > ___ > > busybox mailing list > > busybox@busybox.net > > http://lists.busybox.net/mailman/listinfo/busybox > > ___ > busybox mailing list > busybox@busybox.net > http://lists.busybox.net/mailman/listinfo/busybox > ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH 2/2] udhcpc: fixed a TODO in fill_envp using option scanner
Thank you for applying the commits! I started implementing support for RFC 3396, hopefully it will be ready soon. Martin On Mon, 29 Jun 2020 at 08:40, Denys Vlasenko wrote: > Applied, thanks > > On Tue, Jun 23, 2020 at 3:41 PM Martin Lewis > wrote: > > > > fill_envp now iterates over the packet only once instead of a few > hundred times > > using the new option scanner. > > > > Signed-off-by: Martin Lewis > > --- > > networking/udhcp/dhcpc.c | 201 > --- > > 1 file changed, 87 insertions(+), 114 deletions(-) > > > > diff --git a/networking/udhcp/dhcpc.c b/networking/udhcp/dhcpc.c > > index 102178a4f..2669428e1 100644 > > --- a/networking/udhcp/dhcpc.c > > +++ b/networking/udhcp/dhcpc.c > > @@ -386,59 +386,81 @@ static NOINLINE char > *xmalloc_optname_optval(uint8_t *option, const struct dhcp_ > > return ret; > > } > > > > +static void putenvp(llist_t **envp, char *new_opt) > > +{ > > + putenv(new_opt); > > + llist_add_to(envp, new_opt); > > +} > > + > > +static int is_unknown_opt(uint8_t code, const struct dhcp_optflag **dh, > const char **opt_name) > > +{ > > +*opt_name = ""; > > + > > +/* Find the option: > > + * dhcp_optflags is sorted so we stop searching when dh->code >= > code, which is faster > > + * than iterating over the entire array. > > + * Options which don't have a match in dhcp_option_strings[], e.g > DHCP_REQUESTED_IP, > > + * are located after the sorted array, so these entries will never > be reached > > + * and they'll count as unknown options. > > + */ > > +for (*dh = dhcp_optflags; (*dh)->code && (*dh)->code < code; > (*dh)++); > > + > > +if ((*dh)->code == code) > > +*opt_name = nth_string(dhcp_option_strings, (*dh - > dhcp_optflags)); > > + > > +return (*dh)->code > code; > > +} > > + > > /* put all the parameters into the environment */ > > -static char **fill_envp(struct dhcp_packet *packet) > > +static llist_t *fill_envp(struct dhcp_packet *packet) > > { > > - int envc; > > - int i; > > - char **envp, **curr; > > - const char *opt_name; > > - uint8_t *temp; > > - uint8_t overload = 0; > > - > > -#define BITMAP unsigned > > -#define BBITS (sizeof(BITMAP) * 8) > > -#define BMASK(i) (1 << (i & (sizeof(BITMAP) * 8 - 1))) > > -#define FOUND_OPTS(i) (found_opts[(unsigned)i / BBITS]) > > - BITMAP found_opts[256 / BBITS]; > > - > > - memset(found_opts, 0, sizeof(found_opts)); > > - > > - /* We need 7 elements for: > > -* "interface=IFACE" > > -* "ip=N.N.N.N" from packet->yiaddr > > -* "giaddr=IP" from packet->gateway_nip (unless 0) > > -* "siaddr=IP" from packet->siaddr_nip (unless 0) > > -* "boot_file=FILE" from packet->file (unless overloaded) > > -* "sname=SERVER_HOSTNAME" from packet->sname (unless overloaded) > > -* terminating NULL > > -*/ > > - envc = 7; > > - /* +1 element for each option, +2 for subnet option: */ > > - if (packet) { > > - /* note: do not search for "pad" (0) and "end" (255) > options */ > > -//TODO: change logic to scan packet _once_ > > - for (i = 1; i < 255; i++) { > > - temp = udhcp_get_option(packet, i); > > - if (temp) { > > - if (i == DHCP_OPTION_OVERLOAD) > > - overload |= *temp; > > - else if (i == DHCP_SUBNET) > > - envc++; /* for $mask */ > > - envc++; > > - /*if (i != DHCP_MESSAGE_TYPE)*/ > > - FOUND_OPTS(i) |= BMASK(i); > > - } > > - } > > - } > > - curr = envp = xzalloc(sizeof(envp[0]) * envc); > > + char *new_opt; > > + uint8_t *optptr; > > + const struct dhcp_optflag *dh; > > + struct dhcp_scan_state scan_state; > > + const char *opt_name = ""; > > + llist_t *envp = NULL; > > > > - *curr = xasprintf("interface=%
[PATCH] domain_codec: optimize dname_dec and convert_dname
dname_dec: now iterates over the packet only once. convert_dname: remove redundant checks and code shrink. While testing I've noticed that some of the tests didn't compile properly, so I fixed them. function old new delta dname_dec286 267 -19 dname_enc166 143 -23 -- (add/remove: 0/0 grow/shrink: 0/2 up/down: 0/-42) Total: -42 bytes textdata bss dec hex filename 981645 169151872 1000432 f43f0 busybox_old 981603 169151872 1000390 f43c6 busybox_unstripped Signed-off-by: Martin Lewis --- networking/udhcp/domain_codec.c | 157 1 file changed, 79 insertions(+), 78 deletions(-) diff --git a/networking/udhcp/domain_codec.c b/networking/udhcp/domain_codec.c index 752c0a863..092cd6661 100644 --- a/networking/udhcp/domain_codec.c +++ b/networking/udhcp/domain_codec.c @@ -10,10 +10,14 @@ # define _GNU_SOURCE # define FAST_FUNC /* nothing */ # define xmalloc malloc +# define xzalloc(s) calloc(s, 1) +# define xstrdup strdup +# define xrealloc realloc # include # include # include # include +# include #else # include "common.h" #endif @@ -31,81 +35,72 @@ */ char* FAST_FUNC dname_dec(const uint8_t *cstr, int clen, const char *pre) { - char *ret = ret; /* for compiler */ - char *dst = NULL; + char *ret, *end; + unsigned len, crtpos, retpos, depth; - /* We make two passes over the cstr string. First, we compute -* how long the resulting string would be. Then we allocate a -* new buffer of the required length, and fill it in with the -* expanded content. The advantage of this approach is not -* having to deal with requiring callers to supply their own -* buffer, then having to check if it's sufficiently large, etc. -*/ - while (1) { - /* note: "return NULL" below are leak-safe since -* dst isn't allocated yet */ + crtpos = retpos = depth = 0; + len = strlen(pre); + end = ret = xstrdup(pre); + + /* Scan the string once, allocating new memory as needed */ + while (crtpos < clen) { const uint8_t *c; - unsigned crtpos, retpos, depth, len; + c = cstr + crtpos; - crtpos = retpos = depth = len = 0; - while (crtpos < clen) { - c = cstr + crtpos; + if ((*c & NS_CMPRSFLGS) == NS_CMPRSFLGS) { + /* pointer */ + if (crtpos + 2 > clen) /* no offset to jump to? abort */ + goto error; + if (retpos == 0) /* toplevel? save return spot */ + retpos = crtpos + 2; + depth++; + crtpos = ((c[0] << 8) | c[1]) & 0x3fff; /* jump */ + } else if (*c) { + unsigned label_len; + /* label */ + if (crtpos + *c + 1 > clen) /* label too long? abort */ + goto error; + ret = xrealloc(ret, len + *c + 1); + /* \3com ---> "com." */ + end = (char *)mempcpy(ret + len, c + 1, *c); + end[0] = '.'; - if ((*c & NS_CMPRSFLGS) == NS_CMPRSFLGS) { - /* pointer */ - if (crtpos + 2 > clen) /* no offset to jump to? abort */ - return NULL; - if (retpos == 0) /* toplevel? save return spot */ - retpos = crtpos + 2; - depth++; - crtpos = ((c[0] & 0x3f) << 8) | c[1]; /* jump */ - } else if (*c) { - /* label */ - if (crtpos + *c + 1 > clen) /* label too long? abort */ - return NULL; - if (dst) - /* \3com ---> "com." */ - ((char*)mempcpy(dst + len, c + 1, *c))[0] = '.'; - len += *c + 1; - crtpos += *c + 1; + label_len = *c + 1; + len += label_len; + crtpos += label_len; + } else { + /* NUL: end of current domain name */ + if (retpos == 0) {
Re: [PATCH] shrink last_char_is function even more
Please note that my original patch is still smaller: http://lists.busybox.net/pipermail/busybox/2020-June/088026.html I'm not sure whether it's faster, it would be interesting to compare them. On Tue, 7 Jul 2020 at 14:17, Tito wrote: > Hi, > attached you will find a patch that shrinks libbb's last_char_is function. > bloatcheck is: > > function old new delta > last_char_is 53 42 -11 > > -- > (add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-11) Total: -11 > bytes >textdata bss dec hex filename > 981219 169071872 98 f423e busybox_old > 981208 169071872 87 f4233 busybox_unstripped > > Patch for review: > > --- libbb/last_char_is.c.orig 2020-07-05 09:54:24.737931000 +0200 > +++ libbb/last_char_is.c2020-07-06 14:29:27.768898574 +0200 > @@ -11,14 +11,7 @@ > /* Find out if the last character of a string matches the one given */ > char* FAST_FUNC last_char_is(const char *s, int c) > { > - if (s) { > - size_t sz = strlen(s); > - /* Don't underrun the buffer if the string length is 0 */ > - if (sz != 0) { > - s += sz - 1; > - if ((unsigned char)*s == c) > - return (char*)s; > - } > - } > - return NULL; > -} > + if (!s || !*s) return NULL; > +while (*(s + 1)) s++; > + return (*s == c) ? (char *) s : NULL; > +} > > > The alternative version: > > char* FAST_FUNC last_char_is(const char *s, int c) > { > if (!s || !*s) return NULL; > while (*(++s)); > return (*(--s) == c) ? (char *)s : NULL; > } > > that was also posted to the list on my system was bigger: > > function old new delta > last_char_is 53 46 -7 > > -- > (add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-7) Total: -7 > bytes > > Ciao, > Tito > > > P.S.: test program, if you are aware of other corner cases please tell me: > > > > char* last_char_is(const char *s, int c) > { > if (!s || !*s) > return NULL; > while (*(s + 1)) > s++; > return (*s == c) ? (char *) s : NULL; > } > > int main (int argc, char **argv) { > char *c; > int ret = 0; > > printf("Test 1 'prova','a' : "); > c = last_char_is("prova", 'a'); > if (c != NULL && *c == 'a') { > puts("PASSED"); > } else { > puts("FAILED"); > ret |= 1; > } > printf("Test 2 '' ,'x' : "); > c = last_char_is("", 'x'); > if (c != NULL) { > puts("FAILED"); > ret |= 1; > > } else { > puts("PASSED"); > } > printf("Test 3 NULL ,'3' : "); > c = last_char_is(NULL, 'e'); > if (c != NULL) { > puts("FAILED"); > ret |= 1; > } else { > puts("PASSED"); > } > printf("Test 4 'prova','x' : "); > c = last_char_is("prova", 'x'); > if (c != NULL) { > puts("FAILED"); > ret |= 1; > } else { > puts("PASSED"); > } > printf("Test 5 'prova','\\n': "); > c = last_char_is("prova", '\n'); > if (c != NULL) { > puts("FAILED"); > ret |= 1; > } else { > puts("PASSED"); > } > printf("Test 6 'prova','\\0': "); > c = last_char_is("prova", 0); > if (c != NULL) { > puts("FAILED"); > ret |= 1; > } else { > puts("PASSED"); > } > return ret; > } > > > ___ > busybox mailing list > busybox@busybox.net > http://lists.busybox.net/mailman/listinfo/busybox > ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH 3/4] lsscsi: code shrink and refactor
Here's the struct as specified in the kernel: /* From /drivers/scsi/scsi_sysfs.c in the kernel: * sdev_rd_attr (type, "%d\n"); * sdev_rd_attr (vendor, "%.8s\n"); * sdev_rd_attr (model, "%.16s\n"); * sdev_rd_attr (rev, "%.4s\n"); */ As you can see, type is always printed with %d which should always be a valid signed integer. Could you please give an example of a type for which atoi is not sufficient? The usage of errno and bb_strtoi increases the size of the binary, so unless it's required I don't think it should be used. Martin On Fri, 3 Jul 2020 at 14:45, Markus Gothe wrote: > As the original author of the applet I don't want to see atoi() calls in > the code. You know what atoi() returns when it fails? Same as atoi("0"). > > Please use bb_stroi() for signed integers. > > BR, > Markus > > Sent from my BlackBerry - the most secure mobile device > > > Original Message > > > From: martin.lewis@gmail.com > Sent: June 11, 2020 15:45 > To: busybox@busybox.net > Subject: [PATCH 3/4] lsscsi: code shrink and refactor > > > Remove the use of strou because scsi device types are signed (as seen in > the kernel's code). > Improve the representation of SCSI_DEVICE_LIST. > > function old new delta > .rodata 159915 159920 +5 > lsscsi_main 364 349 -15 > > -- > (add/remove: 0/0 grow/shrink: 1/1 up/down: 5/-15) Total: -10 > bytes >text data bss dec hex filename > 981332 169151872 1000119 f42b7 busybox_old > 981322 169151872 1000109 f42ad busybox_unstripped > > Signed-off-by: Martin Lewis > --- > miscutils/lsscsi.c | 50 ++ > 1 file changed, 34 insertions(+), 16 deletions(-) > > diff --git a/miscutils/lsscsi.c b/miscutils/lsscsi.c > index f737d33d9..e29bedd77 100644 > --- a/miscutils/lsscsi.c > +++ b/miscutils/lsscsi.c > @@ -25,6 +25,28 @@ > > #include "libbb.h" > > +#define SCSI_DEVICE_TYPE(type, name) type name "\0" > +#define SCSI_DEVICE_TYPE_LIST \ > + SCSI_DEVICE_TYPE("\x00", "disk") \ > + SCSI_DEVICE_TYPE("\x01", "tape") \ > + SCSI_DEVICE_TYPE("\x02", "printer") \ > + SCSI_DEVICE_TYPE("\x03", "process") \ > + SCSI_DEVICE_TYPE("\x04", "worm") \ > + SCSI_DEVICE_TYPE("\x06", "scanner") \ > + SCSI_DEVICE_TYPE("\x07", "optical") \ > + SCSI_DEVICE_TYPE("\x08", "mediumx") \ > + SCSI_DEVICE_TYPE("\x09", "comms") \ > + SCSI_DEVICE_TYPE("\x0c", "storage") \ > + SCSI_DEVICE_TYPE("\x0d", "enclosu") \ > + SCSI_DEVICE_TYPE("\x0e", "sim dsk") \ > + SCSI_DEVICE_TYPE("\x0f", "opti rd") \ > + SCSI_DEVICE_TYPE("\x10", "bridge") \ > + SCSI_DEVICE_TYPE("\x11", "osd") \ > + SCSI_DEVICE_TYPE("\x12", "adi") \ > + SCSI_DEVICE_TYPE("\x1e", "wlun") \ > + SCSI_DEVICE_TYPE("\x1f", "no dev") > + > + > static const char scsi_dir[] ALIGN1 = "/sys/bus/scsi/devices"; > > static char *get_line(const char *filename, char *buf, unsigned *bufsize_p) > @@ -59,6 +81,13 @@ int lsscsi_main(int argc UNUSED_PARAM, char **argv > UNUSED_PARAM) > > dir = xopendir("."); > while ((de = readdir(dir)) != NULL) { > + /* > + * From /drivers/scsi/scsi_sysfs.c in the kernel: > + * sdev_rd_attr (type, "%d\n"); > + * sdev_rd_attr (vendor, "%.8s\n"); > + * sdev_rd_attr (model, "%.16s\n"); > + * sdev_rd_attr (rev, "%.4s\n"); > + */ > char buf[256]; > char *ptr; > unsigned bufsize; > @@ -67,7 +96,7 @@ int lsscsi_main(int argc UNUSED_PARAM, char **argv > UNUSED_PARAM) > const char *type_name; > const char *model; > const char *rev; > - unsigned type; > + int type; > > if (!isdigit(de->d_name[0])) > continue; > @@ -88,23 +117,12 @@ int lsscsi_main(int argc UNUSED_PARAM, char **argv > UNUSED_PARAM) > > printf("[%s]\t", de->d_name); > > -#define scsi_device_types \ > - "disk\0""tape\0""printer\0" "process\0" \ > - "worm\0""\0""scanner\0" "optical\0" \ > - "mediumx\0" "comms\0
[PATCH] udhcpc: add support for long options
Duplicate options are currently overridden (only the last option is kept). This leads to unexpected behavior when using long options. The patch adds support for long options in compliance with RFC 3396. Fixes #13136. function old new delta udhcp_run_script 704 765 +61 optitem_unset_env_and_free - 31 +31 static.xmalloc_optname_optval837 833 -4 putenvp 60 52 -8 -- (add/remove: 1/0 grow/shrink: 1/2 up/down: 92/-12) Total: 80 bytes textdata bss dec hex filename 994091 169391872 1012902 f74a6 busybox_old 994171 169391872 1012982 f74f6 busybox_unstripped Signed-off-by: Martin Lewis --- networking/udhcp/dhcpc.c | 125 ++- networking/udhcp/dhcpc.h | 1 + 2 files changed, 93 insertions(+), 33 deletions(-) diff --git a/networking/udhcp/dhcpc.c b/networking/udhcp/dhcpc.c index 50dfead63..d4a7a825e 100644 --- a/networking/udhcp/dhcpc.c +++ b/networking/udhcp/dhcpc.c @@ -115,6 +115,13 @@ enum { /*** Script execution code ***/ +struct dhcp_optitem +{ + unsigned len; + uint8_t *data; + char *env; + uint8_t code; +}; /* get a rough idea of how long an option will be (rounding up...) */ static const uint8_t len_of_option_as_string[] ALIGN1 = { @@ -186,15 +193,15 @@ static int good_hostname(const char *name) #endif /* Create "opt_name=opt_value" string */ -static NOINLINE char *xmalloc_optname_optval(uint8_t *option, const struct dhcp_optflag *optflag, const char *opt_name) +static NOINLINE char *xmalloc_optname_optval(const struct dhcp_optitem *opt_item, const struct dhcp_optflag *optflag, const char *opt_name) { unsigned upper_length; int len, type, optlen; char *dest, *ret; + uint8_t *option; - /* option points to OPT_DATA, need to go back to get OPT_LEN */ - len = option[-OPT_DATA + OPT_LEN]; - + option = opt_item->data; + len = opt_item->len; type = optflag->flags & OPTION_TYPE_MASK; optlen = dhcp_option_lengths[type]; upper_length = len_of_option_as_string[type] @@ -386,11 +393,50 @@ static NOINLINE char *xmalloc_optname_optval(uint8_t *option, const struct dhcp_ return ret; } -static void putenvp(llist_t **envp, char *new_opt) +/* Used by static options (interface, siaddr, etc) */ +static void putenvp(char *new_opt) { + struct dhcp_optitem *opt_item; + opt_item = xzalloc(sizeof(struct dhcp_optitem)); + /* Set opt_item->code = 0, so it won't appear in concat_option's lookup. */ + opt_item->env = new_opt; putenv(new_opt); - log2(" %s", new_opt); - llist_add_to(envp, new_opt); + llist_add_to(&client_data.envp, opt_item); +} + +/* Support RFC3396 Long Encoded Options */ +static struct dhcp_optitem *concat_option(uint8_t *data, uint8_t len, uint8_t code) +{ + llist_t *item; + struct dhcp_optitem *opt_item; + unsigned new_len = len; + + /* Check if an option with the code already exists. +* A possible optimization is to create a bitmap of all existing options in the packet, +* and iterate over the option list only if they exist. +* This will result in bigger code, and because dhcp packets don't have too many options it +* shouldn't have a big impact on performance. +*/ + for (item = client_data.envp; item != NULL; item = item->link) { + opt_item = (struct dhcp_optitem *)item->data; + if (opt_item->code == code) + break; + } + + if (item) { + /* This is a duplicate, concatenate data according to RFC 3396 */ + new_len += opt_item->len; + } else { + /* This is a new option, add a new dhcp_optitem to the list */ + opt_item = xzalloc(sizeof(struct dhcp_optitem)); + opt_item->code = code; + llist_add_to(&client_data.envp, opt_item); + } + + opt_item->data = xrealloc(opt_item->data, new_len); /* xrealloc on the first occurrence (NULL) will call malloc */ + memcpy(opt_item->data + opt_item->len, data, len); + opt_item->len = new_len; + return opt_item; } static const char* get_optname(uint8_t code, const struct dhcp_optflag **dh) @@ -412,50 +458,58 @@ static const char* get_optname(uint8_t code, const struct dhcp_optflag **dh) } /* put all the parameters into the environment */ -static llist_t *fill_envp(struct dhcp_packet *packet) +static void fill_envp(struct dhcp_packet *packet) { uint8_t
Re: [PATCH] udhcpc: add support for long options
Tested both regular options and long options against ISC's dhcpd, looks good. Martin On Thu, 13 Aug 2020 at 10:00, Denys Vlasenko wrote: > On Tue, Aug 4, 2020 at 5:25 PM Martin Lewis > wrote: > > Duplicate options are currently overridden (only the last option is > kept). > > This leads to unexpected behavior when using long options. > > > > The patch adds support for long options in compliance with RFC 3396. > > > > Fixes #13136. > > > > function old new delta > > udhcp_run_script 704 765 +61 > > optitem_unset_env_and_free - 31 +31 > > static.xmalloc_optname_optval837 833 -4 > > putenvp 60 52 -8 > > > -- > > (add/remove: 1/0 grow/shrink: 1/2 up/down: 92/-12) Total: 80 > bytes > >textdata bss dec hex filename > > 994091 169391872 1012902 f74a6 busybox_old > > 994171 169391872 1012982 f74f6 busybox_unstripped > > > > Signed-off-by: Martin Lewis > > --- > > networking/udhcp/dhcpc.c | 125 > ++- > > networking/udhcp/dhcpc.h | 1 + > > 2 files changed, 93 insertions(+), 33 deletions(-) > > > > diff --git a/networking/udhcp/dhcpc.c b/networking/udhcp/dhcpc.c > > index 50dfead63..d4a7a825e 100644 > > --- a/networking/udhcp/dhcpc.c > > +++ b/networking/udhcp/dhcpc.c > > @@ -115,6 +115,13 @@ enum { > > > > > > /*** Script execution code ***/ > > +struct dhcp_optitem > > +{ > > + unsigned len; > > + uint8_t *data; > > + char *env; > > + uint8_t code; > > +}; > > > > /* get a rough idea of how long an option will be (rounding up...) */ > > static const uint8_t len_of_option_as_string[] ALIGN1 = { > > @@ -186,15 +193,15 @@ static int good_hostname(const char *name) > > #endif > > > > /* Create "opt_name=opt_value" string */ > > -static NOINLINE char *xmalloc_optname_optval(uint8_t *option, const > struct dhcp_optflag *optflag, const char *opt_name) > > +static NOINLINE char *xmalloc_optname_optval(const struct dhcp_optitem > *opt_item, const struct dhcp_optflag *optflag, const char *opt_name) > > { > > unsigned upper_length; > > int len, type, optlen; > > char *dest, *ret; > > + uint8_t *option; > > > > - /* option points to OPT_DATA, need to go back to get OPT_LEN */ > > - len = option[-OPT_DATA + OPT_LEN]; > > - > > + option = opt_item->data; > > + len = opt_item->len; > > type = optflag->flags & OPTION_TYPE_MASK; > > optlen = dhcp_option_lengths[type]; > > upper_length = len_of_option_as_string[type] > > @@ -386,11 +393,50 @@ static NOINLINE char > *xmalloc_optname_optval(uint8_t *option, const struct dhcp_ > > return ret; > > } > > > > -static void putenvp(llist_t **envp, char *new_opt) > > +/* Used by static options (interface, siaddr, etc) */ > > +static void putenvp(char *new_opt) > > { > > + struct dhcp_optitem *opt_item; > > + opt_item = xzalloc(sizeof(struct dhcp_optitem)); > > + /* Set opt_item->code = 0, so it won't appear in concat_option's > lookup. */ > > + opt_item->env = new_opt; > > putenv(new_opt); > > - log2(" %s", new_opt); > > - llist_add_to(envp, new_opt); > > + llist_add_to(&client_data.envp, opt_item); > > +} > > This removed logging of environment > > > > +/* Support RFC3396 Long Encoded Options */ > > +static struct dhcp_optitem *concat_option(uint8_t *data, uint8_t len, > uint8_t code) > > +{ > > + llist_t *item; > > + struct dhcp_optitem *opt_item; > > + unsigned new_len = len; > > + > > + /* Check if an option with the code already exists. > > +* A possible optimization is to create a bitmap of all existing > options in the packet, > > +* and iterate over the option list only if they exist. > > +* This will result in bigger code, and because dhcp packets > don't have too many options it > > +* shouldn't have a big impact on performance. > > +*/ > > + for (item = client_data.envp; item != NULL