RFC: first stab at getting rid of libc_hidden_proto() forest

Denys Vlasenko vda.linux at googlemail.com
Sat Apr 12 02:07:07 UTC 2008


Hi,

I want to make clear that I do not intend to apply this patch
without getting a positive review. I don't want people to
start thinking "omg, vda can commit this %&&$ and mess up
the tree"!

I won't. It's really an RFC.

What it does:

I find the need of having libc_hidden_proto(fprintf)
anywhere you want to use fprintf efficiently (i.e.
to call it directly, not thru GOT/PLT) to be unmaintainable.

First, it clogs up the source.

Second, it's too easy to forget to do it, and you get
suboptimal code, silently.

Third, sometimes it gets even more ugly: you need to know
_implementation details_ and dance with #defines like this:

#ifdef __UCLIBC_HAS_XLOCALE__
libc_hidden_proto(__ctype_b_loc)
#elif __UCLIBC_HAS_CTYPE_TABLES__
libc_hidden_proto(__ctype_b)
#endif

Fifth: it needs to be repeated in every .c file where you want
to use e.g. ctype.h functions!

Obvious solution of adding libc_hidden_proto's to .h files
does not work so well since we'd like to not pollute them with
machinery which doesn't belong to public interface.

In the attached patch, I chose to create a "master include file"
which includes almost all other headers; and then marks
all exported functions(variables) with libc_hidden_proto.
The file is called uClibc_libc.h.

Currently it does all #includes first, and all libc_hidden_proto's
next. Maybe it makes sense to group them by header instead:
#include <stdio.h>
libc_hidden_proto(printf)
libc_hidden_proto(sprintf)
...
#include <stdlib.h>
...

Anyway. It is meant to be used this way: you include only this file,
and you are getting (almost) all headers included for you,
and all exported functions libc_hidden_proto'ed.
You don't need to do it again and again in each .c file.
Lots and lots of #includes and libc_hidden_proto's can be removed;
and a lot of churn in adding/removing thme in future is eliminated
too.

The upside is less crowded .c files.

The downside is a somewhat longer build time.

The patch demonstrates this approach by performing this surgery on
a handful of files, namely:
libc/inet/getaddrinfo.c
libc/inet/ifaddrs.c
libc/inet/resolv.c
libc/inet/rpc/pmap_rmt.c
libc/inet/rpc/rcmd.c
libc/misc/locale/locale.c
libc/misc/syslog/syslog.c
libc/misc/time/time.c
libc/misc/wchar/wchar.c
libc/misc/wordexp/wordexp.c
libc/stdio/_scanf.c
libc/stdio/_vfprintf.c

Of these, only libc/misc/locale/locale.c required a little more
work to make it build, and a small fix in libc/inet/netlinkaccess.h
was needed too.

I tried building uclibc with several .configs and fixed all fallout;
then I compared object files one-by-one before and after the patch.

They seem to be identical except for libc/misc/time/tzset.os.
I tracked the difference to this code:

void tzset(void)
{
    _time_tzset((time(NULL)) < new_rule_starts);
}

Before the patch, we were forgetting to treat time()
with libc_hidden_proto(time) macro, and it was called
thru indirection machinery:

 00000000 <__GI_tzset>:
-   0:  53                      push   %ebx
-   1:  e8 00 00 00 00          call   6 <__GI_tzset+0x6>
-   6:  5b                      pop    %ebx
-   7:  81 c3 03 00 00 00       add    $0x3,%ebx
-                       9: R_386_GOTPC  _GLOBAL_OFFSET_TABLE_
-   d:  6a 00                   push   $0x0
-   f:  e8 fc ff ff ff          call   10 <__GI_tzset+0x10>
-                       10: R_386_PLT32 time

+   0:  6a 00                   push   $0x0
+   2:  e8 fc ff ff ff          call   3 <__GI_tzset+0x3>
+                       3: R_386_PC32   __GI_time

As you see, after the patch it was automatically using
faster calling method.

So I found one missed optimization by editing 12 files only.
There are probably more of these.

Please let me know what do you think about this approach.
--
vda
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 6.patch
Type: text/x-diff
Size: 60797 bytes
Desc: not available
Url : http://lists.busybox.net/pipermail/uclibc/attachments/20080412/96bf08c3/attachment.bin 


More information about the uClibc mailing list