On Thursday 18 June 2009 01:43:29 Denys Vlasenko wrote:
> On Wednesday 17 June 2009 23:55, Rob Landley wrote:
> > > I do not want to translate anything. Its a huge PITA with no real
> > > gain. Command line users already know English well enough
> > > to understand the simple subset of it used in messages.
> >
> > Sounds good to me.
> >
> > > > which doesn't seem like something
> > > > you'd want to do to a search path.
> > >
> > > Thus, messages.c is more like strings.c.
> >
> > I still find its existence a bit confusing.  The first string in there,
> > bb_msg_memory_exhausted, exists so things like ash can reimplement
> > xmalloc() and xrealloc() and xstrdup() and friends.  I'm really not sure
> > why they do that, considering that even on a 32 bit system allocation
> > calls will only ever return 0 if you've exhausted _virtual_ address
> > space, and a lack of _physical_ memory will get detected later at page
> > fault time by the OOM killer, so it's not a very useful indicator of
> > anything.  (Unless ash is going to start supporting nommu systems soon? 
> > Even so, why does it have a different mechanism than the rest of busybox?
> >  Historical reasons?)
> >
> > Common messages like that make me think "common code is being
> > reimplemented in multiple places".  Otherwise why would multiple places
> > need to use the same messages?
>
> Because ash historically has some funky allocator (I know nothing
> about it, don't ask).

It's needed for xmalloc() and xrealloc(), and really everything else should 
probably be using those.

An idea I had for toybox (and never quite needed enough to implement) was 
having a global pointer to a setjmp/longjmp buffer, and then have the 
error_msg_and_die() code do something like:

  if (global_jmp_ptr_thingy) longjmp(global_jmp_ptr_thingy, 1);
  else exit(error_exit);

So it was NULL (and since global variables are allocated in the bss it would 
default to NULL without special effort) then the error_msg_and_die() stuff 
would 
kill the program normally, and if it was _not_ null, it would longjmp to that 
location instead.

This way ash wouldn't need its own special allocation functions, but the 
overhead in the common code paths would be a single pointer global, and one 
test and function call.

That said, I haven't taken a close look at what ash is doing either.  Making 
recovery ability _truly_ generic (so you could do general nofork applets) 
would involve recording all memory allocations and file handles and mmaps and 
such so they could be unwound, which is unpleasant overhead.  I assume ash 
already has something like that, though.

> xsetenv() is another good example where it is needed:

Um, not really.  Back when I was doing research for bbsh I learned that 
preventing environment variables from leaking memory is more complicated than 
it seems, and wrote up a long blog entry about it:

  http://landley.net/notes-2006.html#24-10-2006

So really, xsetenv() should probably also be using xmalloc() internally, if 
you want to avoid leaking memory in long-running shell scripts, meaning it 
shouldn't reproduce the error message.

> > > Because I want to cheat:
> >
> > Always a good reason. :)
> >
> > > bb_PATH_root_path + sizeof("PATH=/sbin:/usr/sbin") is a non-root PATH.
> >
> > It might be nice to add a comment in the source code acknowledging that
> > _PATH_STDPATH exists and we've chosen not to use it because XXX.
>
> It is sort-of-documented: include/libbb.h:
>
> /* util-linux manpage says /sbin:/bin:/usr/sbin:/usr/bin,
>  * but I want to save a few bytes here */
> extern const char bb_PATH_root_path[]; /*
> "PATH=/sbin:/usr/sbin:/bin:/usr/bin" */ #define bb_default_root_path
> (bb_PATH_root_path + sizeof("PATH"))
> #define bb_default_path      (bb_PATH_root_path +
> sizeof("PATH=/sbin:/usr/sbin"))

I was suggesting updating the comment to mention _PATH_STDPATH in the headers 
and why we're not using it.

Rob
-- 
Latency is more important than throughput. It's that simple. - Linus Torvalds
_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox

Reply via email to