Re: [PATCH 1/2] Revert ash: use alloca to get rid of setjmp
On Wed, 2015-07-22 at 14:29 +0200, Laurent Bercot wrote: I don't use alloca, but I'm a big fan of VLAs Unfortunately VLAs are significantly less useful (IMO) because they have block scope instead of function scope, like alloca(). ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH 1/2] Revert ash: use alloca to get rid of setjmp
2015-07-22 5:19 GMT+02:00 Rich Felker dal...@libc.org: On Sun, Jul 19, 2015 at 11:07:13PM +0200, Denys Vlasenko wrote: I would rather keep it. What is the most horrible thing which can happen here? Arbitrary code execution due to stack overflow. Does this really need a PoC? alloca is _always_ unsafe unless the argument is bounded and tiny. It would interesting to know if ash ever automatically runs its tokenizer over environment variables. If the tokenizer can only run on the command stream then there's not much to be gained from overflowing the stack since anyone who can inject an evil token in to command stream already has shell access. Daniel. ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH v2] less: improvements to verbose status messages
Ron Yorston wrote: Better, though, might be to put the test in open_file_and_read_lines, where all input files are opened. This would prevent read_lines from trying to read non-regular files. Actually, my concern that read_lines might react badly to being fed /dev/zero was misplaced. read_lines doesn't deal in lines as we know them, it handles terminal-width-sized partial lines, and it inserts a fake EOF after it's read MAXLINES of these things. So even if its input has no newlines and no EOF read_lines will eventually terminate. I still think it would be better to detect non-regular files earlier and I'll put together a patch for that. The real 'less' does this: [rmy@vulcan ~]$ less /dev/zero /dev/zero is not a regular file (use -f to see it) [rmy@vulcan ~]$ Interestingly if you try -f it doesn't work as well as BusyBox less. 'less -f /dev/zero' is OK but 'less -f -S /dev/zero' loops forever (or until it fills memory, I killed it after 2GB). Ron ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] sync: support the -d and -f flags and take files as arguments
On Tue, 2015-07-21 at 19:52 +0200, Denys Vlasenko wrote: I reworked it so that these additions are optional, and applied the result. Looks great! I was a bit worried about the bloat-o-meter values, but you were able to shrink the changes down considerably. Just one minor thing that I noticed. When running busybox sync -f or busybox sync -d, that is, with an option but no files, sync() is called. This may be a bit unexpected and can easily result from something like busybox sync -f `ls` in a shell script in an empty directory (not that this is good style). Is this behavior what you intended? Thanks! Thank you. On Mon, Jul 20, 2015 at 6:55 PM, Ari Sundholm a...@tuxera.com wrote: From: Ari Sundholm a...@tuxera.com This brings busybox in line with modern coreutils sync. function old new delta sync_main 19 214+195 .rodata 155261 155373+112 packed_usage 30228 30270 +42 -- (add/remove: 0/0 grow/shrink: 3/0 up/down: 349/0) Total: 349 bytes Signed-off-by: Ari Sundholm a...@tuxera.com --- coreutils/Config.src | 6 coreutils/sync.c | 87 2 files changed, 81 insertions(+), 12 deletions(-) diff --git a/coreutils/Config.src b/coreutils/Config.src index 1ec3a0a..02155d2 100644 --- a/coreutils/Config.src +++ b/coreutils/Config.src @@ -571,12 +571,6 @@ config SUM help checksum and count the blocks in a file -config SYNC - bool sync - default y - help - sync is used to flush filesystem buffers. - config TAC bool tac default y diff --git a/coreutils/sync.c b/coreutils/sync.c index 7d98a1e..c5fdc5e 100644 --- a/coreutils/sync.c +++ b/coreutils/sync.c @@ -3,16 +3,40 @@ * Mini sync implementation for busybox * * Copyright (C) 1995, 1996 by Bruce Perens br...@pixar.com. + * Copyright (C) 2015 by Ari Sundholm a...@tuxera.com * * Licensed under GPLv2 or later, see file LICENSE in this source tree. */ /* BB_AUDIT SUSv3 N/A -- Matches GNU behavior. */ +//config:config SYNC +//config: bool sync +//config: default y +//config: help +//config:sync is used to flush filesystem buffers. +//config:config FEATURE_SYNC_SYNCFS +//config: bool Enable the use of syncfs(2) +//config: default y +//config: depends on SYNC +//config: help +//config:Enables the sync applet to use syncfs(2) to offer the additional +//config:-f flag, which allows for synchronizing the filesystems underlying +//config:a set of files. //usage:#define sync_trivial_usage -//usage: +//usage: [-d +//usage: IF_FEATURE_SYNC_SYNCFS( +//usage:|-f +//usage: ) +//usage: ] [FILE ...] //usage:#define sync_full_usage \n\n -//usage: Write all buffered blocks to disk +//usage: Write all buffered blocks in FILEs or all filesystems to disk +//usage: \n -d Avoid syncing metadata +//usage:IF_FEATURE_SYNC_SYNCFS( +//usage: \n -f Sync underlying filesystem +//usage:) +//usage: + #include libbb.h @@ -21,10 +45,61 @@ int sync_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE; int sync_main(int argc UNUSED_PARAM, char **argv IF_NOT_DESKTOP(UNUSED_PARAM)) { - /* coreutils-6.9 compat */ - bb_warn_ignoring_args(argv[1]); + unsigned opts; + int ret = EXIT_SUCCESS; + + enum { + OPT_DATASYNC = (1 0), +#if ENABLE_FEATURE_SYNC_SYNCFS + OPT_SYNCFS = (1 1), +#endif + }; + + opt_complementary = +#if ENABLE_FEATURE_SYNC_SYNCFS + d--f:f--d:d--d:f--f; +#else + d--d; +#endif + opts = getopt32(argv, + d +#if ENABLE_FEATURE_SYNC_SYNCFS + f +#endif + ); + + argv += optind; + + /* Handle the no-argument case. */ + if (!*argv !(opts OPT_DATASYNC) +#if ENABLE_FEATURE_SYNC_SYNCFS +!(opts OPT_SYNCFS) +#endif + ) + sync(); + + while (*argv) { + int fd = open(*argv, O_RDONLY); - sync(); + if (fd 0) { + bb_perror_msg(%s: open, *argv); + ret = EXIT_FAILURE; + } else { + if (opts OPT_DATASYNC fdatasync(fd) 0) { + bb_perror_msg(%s: fdatasync, *argv); + ret = EXIT_FAILURE;
[PATCH] fbsplash: use virtual y size in mmap size calculations
The virtual y can be larger - and we can be even writing there since we are taking into account the y offset. Avoids possible crash. But use it only if set, seems it is not set if virtual area is not allocated (though, often fbcon allocates some scrollback area). Signed-off-by: Timo Teräs timo.te...@iki.fi --- miscutils/fbsplash.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/miscutils/fbsplash.c b/miscutils/fbsplash.c index 7b695b2..e05ab3b 100644 --- a/miscutils/fbsplash.c +++ b/miscutils/fbsplash.c @@ -150,7 +150,7 @@ static void fb_open(const char *strfb_device) // map the device in memory G.addr = mmap(NULL, - G.scr_var.yres * G.scr_fix.line_length, + (G.scr_var.yres_virtual ?: G.scr_var.yres) * G.scr_fix.line_length, PROT_WRITE, MAP_SHARED, fbfd, 0); if (G.addr == MAP_FAILED) bb_perror_msg_and_die(mmap); -- 2.2.0 ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH 1/2] Revert ash: use alloca to get rid of setjmp
On 22/07/2015 13:46, Xabier Oneca -- xOneca wrote: I've read alloca is not portable anyways... It's probably portable on every platform busybox is running on. (I'm not advertising its use, just saying I don't think it's the right argument for not using it.) I'm not an expert, but why not just use plain ol' malloc? malloc uses the heap. It's more complicated, pulls in more code, takes more time to run, and requires manual management; whereas alloca and VLAs use the stack, which is smaller, simpler and faster. For a minimalistic piece of software such as busybox, it makes sense to avoid using the heap when it can be avoided. I don't use alloca, but I'm a big fan of VLAs; they save me lots of calls to malloc, and a good portion of the programs I write don't use the heap at all (which is a nice way to ensure they will never leak memory). However, like Denys, I thought that programs simply crashed when the kernel couldn't allocate them enough stack; on MMU systems, there's no reason why the kernel can't enforce a bound, or simply let the stack run unbounded. Rich (or anyone), any good pointers to read about this ? -- Laurent ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH 1/2] Revert ash: use alloca to get rid of setjmp
Xabier Oneca wrote: I'm not an expert, but why not just use plain ol' malloc? That's what the code in question did originally. This is in the shell parser which is recursive and has to allow errors to propagate upwards. The code in this case had setjmp/longjmp calls to avoid leaking allocated memory when a parse error was detected at a lower level. This made the code more obscure (and bigger!). Using alloca allows the memory to be freed automatically when the function returns and results in simpler (and smaller!) code. Ron ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: osuosl.org and Spamhaus PBL
X-Greylist: delayed 00:06:59 by SQLgrey-1.7.6 (For the record, I read the headers wrong, and it's osuosl.org that actually performs that greylisting. My apologies to Numericable, for once I accused them wrongly. Now to understand what that greylisting does for osuosl.org... if they're using PBL in the first place, and only accept SMTP connections from known ISP servers... oh well, some things are best left unexplained.) -- Laurent ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH v2] less: improvements to verbose status messages
Ron Yorston wrote: I still think it would be better to detect non-regular files earlier and I'll put together a patch for that. Well, it didn't turn out to be as clear-cut as I'd hoped. Patch follows: take it or leave it. Ron ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
[PATCH] less: rearrange detection of non-regular files
Move the code to detect non-regular files to the point where the file is being opened. The line-counting code in m_status_print now only has to handle regular files. Still check the error return from the open call in m_status_print. This permits detection of the case where a file becomes unreadable between it first being opened and the call to m_status_print. But mark the file as being non-regular so we don't try that again. function old new delta reinitialize 197 245 +48 m_status_print 409 379 -30 -- (add/remove: 0/0 grow/shrink: 1/1 up/down: 48/-30) Total: 18 bytes Signed-off-by: Ron Yorston r...@pobox.com --- miscutils/less.c | 21 ++--- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/miscutils/less.c b/miscutils/less.c index dd932c5..6a59a00 100644 --- a/miscutils/less.c +++ b/miscutils/less.c @@ -167,7 +167,8 @@ enum { pattern_valid = 0 }; enum { READING_FILE = -1, - READING_STDIN = -2 + READING_STDIN = -2, + READING_NONREG = -3 }; struct globals { @@ -633,15 +634,15 @@ static void m_status_print(void) int count, fd; ssize_t len, i; char buf[4096]; - struct stat stbuf; - /* count number of lines in file */ + /* count number of lines in (regular) file */ count = 0; fd = open(filename, O_RDONLY); - if (fd 0) + if (fd 0) { + /* somebody stole my file! */ + num_lines = READING_NONREG; goto skip; - if (fstat(fd, stbuf) != 0 || !S_ISREG(stbuf.st_mode)) - goto do_close; + } while ((len = safe_read(fd, buf, sizeof(buf))) 0) { for (i = 0; i len; ++i) { if (buf[i] == '\n' ++count == MAXLINES) @@ -650,7 +651,6 @@ static void m_status_print(void) } done: num_lines = count; - do_close: close(fd); skip: ; } @@ -939,6 +939,13 @@ static void buffer_line(int linenum) static void open_file_and_read_lines(void) { if (filename) { +#if ENABLE_FEATURE_LESS_FLAGS + struct stat stbuf; + + xstat(filename, stbuf); + if (!S_ISREG(stbuf.st_mode)) + num_lines = READING_NONREG; +#endif xmove_fd(xopen(filename, O_RDONLY), STDIN_FILENO); } else { /* less with no arguments in argv[] */ -- 2.4.3 ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH 1/2] Revert ash: use alloca to get rid of setjmp
Hello, 2015-07-22 5:19 GMT+02:00 Rich Felker dal...@libc.org: On Sun, Jul 19, 2015 at 11:07:13PM +0200, Denys Vlasenko wrote: I would rather keep it. What is the most horrible thing which can happen here? Arbitrary code execution due to stack overflow. Does this really need a PoC? alloca is _always_ unsafe unless the argument is bounded and tiny. Rich I've read alloca is not portable anyways... I'm not an expert, but why not just use plain ol' malloc? Cheers, Xabier Oneca_,,_ ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
[PATCH] modutils: merge module_entry and module_info to common code
This merges the in-memory module info structures of modprobe and depmod. This allows sharing hashing by modulename code improving depmod runtime with almost factor of 2x. Signed-off-by: Timo Teräs timo.te...@iki.fi --- modutils/depmod.c | 99 +++-- modutils/modprobe.c | 66 --- modutils/modutils.c | 43 +++ modutils/modutils.h | 29 4 files changed, 114 insertions(+), 123 deletions(-) diff --git a/modutils/depmod.c b/modutils/depmod.c index 9713aef..b8699ba 100644 --- a/modutils/depmod.c +++ b/modutils/depmod.c @@ -14,6 +14,14 @@ #include modutils.h #include sys/utsname.h /* uname() */ +struct globals { + module_db db; +} FIX_ALIASING; +#define G (*ptr_to_globals) +#define INIT_G() do { \ + SET_PTR_TO_GLOBALS(xzalloc(sizeof(G))); \ +} while (0) + /* * Theory of operation: * - iterate over all modules and record their full path @@ -21,21 +29,12 @@ * for each depends, look through our list of full paths and emit if found */ -typedef struct module_info { - struct module_info *next; - char *name, *modname; - llist_t *dependencies; - llist_t *aliases; - llist_t *symbols; - struct module_info *dnext, *dprev; -} module_info; - static int FAST_FUNC parse_module(const char *fname, struct stat *sb UNUSED_PARAM, - void *data, int depth UNUSED_PARAM) + void *data UNUSED_PARAM, int depth UNUSED_PARAM) { - module_info **first = (module_info **) data; char *image, *ptr; - module_info *info; + module_entry *e; + /* Arbitrary. Was sb-st_size, but that breaks .gz etc */ size_t len = (64*1024*1024 - 4096); @@ -43,17 +42,10 @@ static int FAST_FUNC parse_module(const char *fname, struct stat *sb UNUSED_PARA return TRUE; image = xmalloc_open_zipped_read_close(fname, len); - info = xzalloc(sizeof(*info)); - info-next = *first; - *first = info; + e = moddb_get(G.db, bb_get_last_path_component_nostrip(fname), 1); + e-name = xstrdup(fname + 2); /* skip ./ */ - info-dnext = info-dprev = info; - info-name = xstrdup(fname + 2); /* skip ./ */ - info-modname = filename2modname( - bb_get_last_path_component_nostrip(fname), - NULL - ); for (ptr = image; ptr image + len - 10; ptr++) { if (is_prefixed_with(ptr, depends=)) { char *u; @@ -62,11 +54,11 @@ static int FAST_FUNC parse_module(const char *fname, struct stat *sb UNUSED_PARA for (u = ptr; *u; u++) if (*u == '-') *u = '_'; - ptr += string_to_llist(ptr, info-dependencies, ,); + ptr += string_to_llist(ptr, e-deps, ,); } else if (ENABLE_FEATURE_MODUTILS_ALIAS is_prefixed_with(ptr, alias=) ) { - llist_add_to(info-aliases, xstrdup(ptr + 6)); + llist_add_to(e-aliases, xstrdup(ptr + 6)); ptr += strlen(ptr); } else if (ENABLE_FEATURE_MODUTILS_SYMBOLS is_prefixed_with(ptr, __ksymtab_) @@ -77,7 +69,7 @@ static int FAST_FUNC parse_module(const char *fname, struct stat *sb UNUSED_PARA ) { continue; } - llist_add_to(info-symbols, xstrdup(ptr)); + llist_add_to(e-symbols, xstrdup(ptr)); ptr += strlen(ptr); } } @@ -86,24 +78,13 @@ static int FAST_FUNC parse_module(const char *fname, struct stat *sb UNUSED_PARA return TRUE; } -static module_info *find_module(module_info *modules, const char *modname) -{ - module_info *m; - - for (m = modules; m != NULL; m = m-next) - if (strcmp(m-modname, modname) == 0) - return m; - return NULL; -} - -static void order_dep_list(module_info *modules, module_info *start, - llist_t *add) +static void order_dep_list(module_entry *start, llist_t *add) { - module_info *m; + module_entry *m; llist_t *n; for (n = add; n != NULL; n = n-link) { - m = find_module(modules, n-data); + m = moddb_get(G.db, n-data, 0); if (m == NULL) continue; @@ -118,7 +99,7 @@ static void order_dep_list(module_info *modules, module_info *start, start-dprev = m; /* recurse */ - order_dep_list(modules, start, m-dependencies); + order_dep_list(start, m-deps); } } @@ -184,12 +165,15 @@ enum { int depmod_main(int argc,
[PATCH] depmod: support generating kmod binary index files
This allows to use busybox depmod, and run daemons using libkmod (or even kmod modprobe if needed). About +1500 bytes when enabled. This patch merges some depmod code paths, so when this is disabled it shrinks the code size a little bit. Signed-off-by: Timo Teräs timo.te...@iki.fi --- modutils/Config.src | 10 +++ modutils/depmod.c | 253 +++- modutils/modprobe.c | 15 modutils/modutils.c | 15 modutils/modutils.h | 15 5 files changed, 253 insertions(+), 55 deletions(-) diff --git a/modutils/Config.src b/modutils/Config.src index 449ac65..4acefca 100644 --- a/modutils/Config.src +++ b/modutils/Config.src @@ -232,6 +232,16 @@ config FEATURE_MODUTILS_ALIAS Say Y if unsure. +config FEATURE_MODUTILS_BIN + bool Support for the kmod .bin file format + default n + depends on DEPMOD !MODPROBE_SMALL + select PLATFORM_LINUX + help + Generate kmod compatible binary index files for .dep, .alias, + .symbols and .builtin files. Allows mixing use of busybox + modutils and kmod (binaries and library). + config FEATURE_MODUTILS_SYMBOLS bool Support for module.symbols file default y diff --git a/modutils/depmod.c b/modutils/depmod.c index b8699ba..1516966 100644 --- a/modutils/depmod.c +++ b/modutils/depmod.c @@ -2,7 +2,7 @@ /* * depmod - generate modules.dep * Copyright (c) 2008 Bernhard Reutner-Fischer - * Copyrihgt (c) 2008 Timo Teras timo.te...@iki.fi + * Copyrihgt (c) 2008-2015 Timo Teras timo.te...@iki.fi * Copyright (c) 2008 Vladimir Dronnikov * * Licensed under GPLv2 or later, see file LICENSE in this source tree. @@ -14,8 +14,18 @@ #include modutils.h #include sys/utsname.h /* uname() */ +#define INDEX_MINCHAR 32 +#define INDEX_MAXCHAR 128 + +typedef struct index_node { + char *prefix; + llist_t *values; + struct index_node *children[INDEX_MAXCHAR-INDEX_MINCHAR]; +} index_node; + struct globals { module_db db; + index_node *root_node; } FIX_ALIASING; #define G (*ptr_to_globals) #define INIT_G() do { \ @@ -103,12 +113,6 @@ static void order_dep_list(module_entry *start, llist_t *add) } } -static void xfreopen_write(const char *file, FILE *f) -{ - if (freopen(file, w, f) == NULL) - bb_perror_msg_and_die(can't open '%s', file); -} - //usage:#if !ENABLE_MODPROBE_SMALL //usage:#define depmod_trivial_usage [-n] [-b BASE] [VERSION] [MODFILES]... //usage:#define depmod_full_usage \n\n @@ -162,6 +166,169 @@ enum { OPT_C = (1 9), /* -C,--config etc_modules_conf: ignored */ }; +/* Support for the mod binary index generation */ + +static void index_init(const char *filename) +{ + if (ENABLE_FEATURE_MODUTILS_BIN) { + index_node *n; + + n = xzalloc(sizeof(index_node)); + n-prefix = xstrdup(); + G.root_node = n; + } + + if (filename !(option_mask32 OPT_n)) { + if (freopen(filename, w, stdout) == NULL) + bb_perror_msg_and_die(can't open '%s', filename); + } +} + +static void index_add(const char *key, char *value, const char *prefix) +{ + if (prefix *prefix) + printf(%s%s %s\n, prefix, key, value); + else if (prefix) + printf(%s\n, value); + + if (ENABLE_FEATURE_MODUTILS_BIN) { + index_node *cur = G.root_node, *n; + unsigned i = 0, j, ch; + + while (1) { + /* Ensure node-prefix is a prefix of str[i]. +* If it is not already, then we must split node. */ + for (j = 0; cur-prefix[j]; j++) { + ch = cur-prefix[j]; + if (ch != key[i+j]) { + /* New child is copy of node with prefix[j+1..N] */ + n = xzalloc(sizeof(index_node)); + n-prefix = xstrdup(cur-prefix[j+1]); + n-values = cur-values; + memcpy(n-children, cur-children, sizeof(n-children)); + + /* Parent has prefix[0..j], child at prefix[j] */ + cur-prefix[j] = '\0'; + cur-values = NULL; + memset(cur-children, 0, sizeof(cur-children)); + cur-children[ch-INDEX_MINCHAR] = n; + break; + } + } + i += j; + + ch = key[i]; + if (ch == 0) + break; + + if (ch INDEX_MINCHAR || ch = INDEX_MAXCHAR) +
osuosl.org and Spamhaus PBL
Hello, The latest message I sent to the list was blocked by the busybox.net MX because my IP was listed in the SpamHaus PBL - that means it is a dynamic address provided by my ISP, which it indeed is. Mail servers that reject mail based on a PBL listing basically enforce the use of the provider's SMTP server and disallow use of the customer's own SMTP server. I have my own SMTP server. It works, it has always worked, and if it doesn't work, I can fix it. I do not want to use my ISP's SMTP server for several reasons: - If at some point I want to encrypt my mail, I want it to be done between my server and the recipient's server, not between my server and my ISP's. I don't want to allow my ISP to read my mail. - My ISP is made of incompetent morons who have no idea what customer service is, how the Internet works or how to configure or scale a service. Every mail I send through them sits in their queue for several minutes. The previous mail I sent had this charming header in it: X-Greylist: delayed 00:06:59 by SQLgrey-1.7.6 (so it wasn't actually incompetence in this case, it was intentional disservice.) The PBL is part of a larger tendency to disempower individual users and put that power into the ISP's hands, which comes with a very unsavory smell of surveillance, breach of net neutrality, and also user infantilization and deresponsabilization. It's the first time I get a bounce based on a PBL listing, and the busybox.net MX is the first MX giving me such a bounce. I have to say it is a major disappointment coming from such a list. I noticed in the headers that the osuosl.org servers were actually hosting the list. So it looks like osuosl.org implements PBL blacklisting. It's the first time I send a mail to the Busybox list and it is rejected by PBL, so either the list hosting was switched to osuosl.org very recently, or they added the feature recently. Would it be envisionable to either switch hostings again, or persuade osuosl.org to roll this back (one can hope) ? I want to think that busybox.net people are technically savvy, responsible people who dislike this kind of disempowerment as much as I do, and that my plea will fall on listening, understanding ears. Thanks. (osuosl.org homepage: The Open Source Lab is an organization working for the advancement of open source technologies. They forgot to mention without consideration as to how they are applied.) -- Laurent ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH 1/2] Revert ash: use alloca to get rid of setjmp
2015-07-22 14:29 GMT+02:00 Laurent Bercot ska-dietl...@skarnet.org: On 22/07/2015 13:46, Xabier Oneca -- xOneca wrote: I've read alloca is not portable anyways... It's probably portable on every platform busybox is running on. (I'm not advertising its use, just saying I don't think it's the right argument for not using it.) I'm not an expert, but why not just use plain ol' malloc? malloc uses the heap. It's more complicated, pulls in more code, takes more time to run, and requires manual management; whereas alloca and VLAs use the stack, which is smaller, simpler and faster. So, generated code is smaller and faster with alloca, but at a possible cost of stability? For a minimalistic piece of software such as busybox, it makes sense to avoid using the heap when it can be avoided. I don't use alloca, but I'm a big fan of VLAs; they save me lots of calls to malloc, and a good portion of the programs I write don't use the heap at all (which is a nice way to ensure they will never leak memory). However, like Denys, I thought that programs simply crashed when the kernel couldn't allocate them enough stack; on MMU systems, there's no reason why the kernel can't enforce a bound, or simply let the stack run unbounded. For what I read, VLAs are not safer than alloca... Thanks, Xabier Oneca_,,_ ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH 1/2] Revert ash: use alloca to get rid of setjmp
2015-07-22 14:48 GMT+02:00 Ron Yorston r...@pobox.com: Xabier Oneca wrote: I'm not an expert, but why not just use plain ol' malloc? That's what the code in question did originally. This is in the shell parser which is recursive and has to allow errors to propagate upwards. The code in this case had setjmp/longjmp calls to avoid leaking allocated memory when a parse error was detected at a lower level. This made the code more obscure (and bigger!). Using alloca allows the memory to be freed automatically when the function returns and results in simpler (and smaller!) code. Ron Thanks for the explanation. Now I have read and understand more about setjmp. Didn't know it was used to implement an exception handler... Shell parsing is hell! :S Cheers, Xabier Oneca_,,_ ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox