xfuncs, bb_ funcs and "nofork/noexec" plans

[Denis Vlasenko]

On Friday 10 March 2006 21:57, Rob Landley wrote:
> > Many simpler applets typically do not qualify just because
> > of few calls to xfuncs. It would be useful if we will have
> > xfunc equivalent which prints error message, just like xfunc
> > does, but instead of exiting returns an error.
> >
> > Like this:
> >
> > void *bb_malloc(size_t size)
> > {
> >         void *ptr = malloc(size);
> >         if (ptr == NULL && size != 0)
> >                 bb_error_msg(bb_msg_memory_exhausted);
> >         return ptr;
> > }
> 
> The point of these functions is so the caller doesn't have to check for 
> errors.  If you return, anybody trying to use the returned value will 
> segfault.

I am not advocating _removing_ xmalloc. I meant that we might benefit from
another variation for those applets which we don't want to exit:

int cut_main()
{
        ...
        char *p = bb_malloc(size);
        if (!p)
                /* We do not need to bb_error_msg(bb_msg_memory_exhausted) here,
                 * it's already done for us by bb_malloc!
                 */
		return 1;
        ...
}
	
> > What do you think?
> >
> > Another small matter - doesn't "x" means "will yell & exit on error"
> > in the name of xfunc? Do bb_ functions never exit? We'd better enforce
> > this (or similar) rule in order to make checking above four-point
> > list easier.
> 
> The above list isn't sufficient.  We have to audit the suckers one by one and 
> understand what they're doing, and mark them as NOFORK in applets.h.

My question was "is there a naming convention about xsomething() and bb_something()
to have specific meaning regarding possibility of abrupt exit()?"

> > How would one know that his program can exit here (taken from ftpgetput.c):
> >
> >         server->s_in = &s_in;
> >         bb_lookup_host(&s_in, argv[optind]);
> >         s_in.sin_port = bb_lookup_port(port, "tcp", 21);
> >         if (verbose_flag) {
> >                 printf("Connecting to %s[%s]:%d\n",
> >                                 argv[optind], inet_ntoa(s_in.sin_addr),
> > ntohs(s_in.sin_port)); }
> >
> > It's not immediately obvious!
> 
> You have to be familiar with the code.  And it's done that way to save space.

I do not propose to make code bigger. I basically propose:

-         bb_lookup_host(&s_in, argv[optind]);
+         xlookup_host(&s_in, argv[optind]);

to make it easy to immediately notice: "Aha, this thing can call exit() on error!"

This will make auditing those NOFORK applets easier.

> > Will you accept patches which fix such bb_ fuctions?
> 
> It's not a "fix" if it's bigger.
--
vda