warnings in libbb.h

[Bernhard Fischer]

On Sun, Sep 11, 2005 at 08:37:43PM -0500, Rob Landley wrote:
>On Sunday 11 September 2005 16:15, Bernhard Fischer wrote:
>> Hi,
>>
>> There are a couple of warnings in libbb.h. Sorry if this was brought up
>> before.
>>
>> Can we remove them or how should they be dealt with?
>> Please let me know what you think, TIA.
>
>These comments seem like somebody's "notes to self".  Might be worth checking 
>the history to figure out who that was...

so we can remove this one, good.
->> //#warning rename?

>All the "warning rename" comments seem to be "maybe we should rename this 
>function", but in most cases it looks more like we need to document what they 
>do, which is a general problem for the whole libbb directory.  We need an 
>inventory and documentation of that...
>
>> //#warning is this needed anymore?
>> #ifndef DMALLOC
>> extern void *xmalloc (size_t size);
>> extern void *xrealloc(void *old, size_t size);
>> extern void *xcalloc(size_t nmemb, size_t size);
>>
>> My dmalloc undef's those before it defines them. Also, it is
>> recommended that dmalloc should be included last. Would the attached
>> patch be appropriate (slightly tested with allyesconfig and both malloc
>> and alloc on stack)?
>
>I don't seem to have a dmalloc.h anywhere on my system (ubuntu "hoar-infested 
>hedgehog" release.  Hey, I finally upgraded from knoppix 3.6... :)  What's it 
>for?

It's a memory debugging library. see dmalloc.com

I'll ask mjn3 to apply the patch as he seems to have been the last one
who did touch dmalloc support.
>
>I just ripped out the two #includes and did a clean make allyesconfig, and it 
>built just fine.  What exactly was this needed for in the first place?
>
>> //#warning pitchable now?
>> extern unsigned long bb_xparse_number(const char *numstr,
>>
>> Current users are: stty, dd, losetup
>> It looks a bit like bb_xgetularg_bnd_sfx and friends, so we might use
>> those instead. Haven't looked closely.
>
>Well, for starters losetup could pretty happily use atol(), since -o is just 
>bytes...
>
>>
>> //#warning change names?
>> my_getpwnam, my_getgrnam, my_getug, my_getpwuid, my_getgrgid
>>
>> I'd rename them to bb_*(), fwiw.
>
>bb_xgetgrnam() seems more likely for the first one, since the x generally 
>seems to mean "die if this isn't found"...
>
>Tito was the last guy to play with these. :)

Tito, would you look into renaming them or do you prefer me to look into
making a patch for this?
>
>>
>> //#warning yuk!
>> char *fgets_str(FILE *file, const char *terminating_string);
>
>Compare to bb_get_line_from_file()...

Only dpkg uses fgets_str(), perhaps i'll get to proposing a patch to
remove fgets_str().
>
>> //#warning wrap this?
>> char *dirname (char *path);
>>
>> at least rmdir.c includes libgen.h, so we may put that include just into
>> libbb.h and remove that definition.
>
>Possibly they're trying to avoid the two versions of basename().  (check out 

That might be the case, yes.

>"man 3 dirname"...)  Or maybe they want something that doesn't modify its 
>argument...
>
>Do we really need libbb.h to #include standard headers for the applets?  This 
>isn't something busybox defines, I don't know why we have a prototype for it 
>at all...

The prototype is probably there because of basename() as you say.

I have no *real* opinion on whether or not libbb.h should #include
headers it doesn't strictly need (I'd remove them and add them where
they are actually needed, ymmv).

>>
>> //#warning put these in .o files
>> CURRENT_TTY, CONSOLE_DEV.
>> Maybe messages.c would be a good place to put them.
>
>A) I think those are translated messages.  What's the deal with this #ifdef L_ 
>stuff, anyway?  (Is this automatically generated by the dependency stuff?)

Well, it's currently not created by the dependency stuff (see the note
below referring to vodz' new bb_mkdep)

A c file contains several functions. These are #ifdef L_functionname.
If a .config needs a certain function, it will (not now but hopefully
later on, vodz) depend on it. The compiler will see the libs rule which
says that for
func.o: big.c
	cc -DL_$(notdir $*) -c big.c -o func.o

the "$(notdir $*)" is a long variant of "$(*F)", i.e. the filename of
the stem. This expands to the final
cc -DL_func -c big.c -o func.o

With this approach, we would not need to build all and every objs
provided by the libs only to discard it in the final (static) link
stage of the busybox binary if it isn't referenced.

I asked vodz what he thinks about also scanning for the functions
provided by e.g. libbb.h when building the dependency-list with his new bb_mkdep.
With these dependencies, it would no longer be necessary to build a ton
of stuff when it's not needed anyway. See e.g. make allnoconfig and all
the libs which are built. That part about emitting dependencies of the
helper lib's obj was just a suggestion and would be a nice feature,
imho.
>
>B) Compare mtab_file.c.
>
>I've avoided opening the "libbb cleanup" can of worms so far because, well, 
>it's big...

Yes. I was thinking about gradual small steps for a start.

We can easily put those two defines in a separate file (console.c to be
unimaginative), ok?