picolibc / picolibc

picolibc - a C library designed for embedded 32- and 64- bit systems.
https://keithp.com/picolibc
GNU General Public License v2.0
1.17k stars 175 forks source link

Organize source files to be `grep`-able #859

Open GalaxyShard opened 1 day ago

GalaxyShard commented 1 day ago

I have been porting over picolibc to the Zig build system and found that determining which files end up getting compiled directly is quite difficult.

For example, here is an excerpt of some notes I created showing the logic behind which files are compiled under newlib/libc/stdlib (checked against commit 1584f3abaed2f1071060dfd6e9294a46d1db2bad):

stdlib:
    13+13+4+8+78+14+3+1 = 134
    134/138

    4 not compiled:
        wcstoll_r.c     appears dead
        wcstoull_r.c    appears dead
        calloc.c        appears dead
        mallocr.c       #included by malloc-*.c files

    +13 nano-*.c: compiled if newlib-nano-malloc
    +13 malloc-*.c: compiled if NOT newlib-nano-malloc
    +4 pico-*.c: compiled if picoexit
    +8 compiled if NOT picoexit:
        __atexit.c
        __call_atexit.c
        atexit.c
        cxa_atexit.c
        cxa_finalize.c
        exit.c
        on_exit.c
        quick_exit.c

    +14 compiled if not tinystdio:
        dtoa.c
        ecvtbuf.c
        efgcvt.c
        gdtoa-gethex.c
        gdtoa-hexnan.c
        mprec.c
        strtod.c
        strtodg.c
        strtol.c
        strtoul.c
        strtoll.c
        strtoull.c
        strtoimax.c
        strtoumax.c

        +3 if have_long_double:
            strtold.c
            strtorx.c
            wcstold.c
            +1 if io_long_double:
                ldtoa.c

(full notes)

Looking at, for example, the cases of !picoexit, !tinystdio, they are very special-cased and I imagine it would be easy to accidentally miss that a file was added or deleted from those lists--there are more examples like this throughout the repository, especially in tinystdio. Additionally, it appears there are a handful of files which are completely unused and do not compile.

I think it would be simpler to use folders and/or filename suffixes/prefixes so that a find | grep or some equivalent is all you need to determine which files are compiled under certain conditions. This follows the convention used for the newlib-nano-malloc option--the files which are compiled when it is enabled are all prefixed with "nano-", while the files compiled when it is disabled are all prefixed with "malloc-", making it easy to programmatically find and list the necessary files.

keith-packard commented 1 day ago

Yeah, I continue to hesitate to reorganize the source code as there are still occasional useful patches appearing in the newlib repository. However, removing completely unused code is still a good idea as that doesn't impact importing fixes.

GalaxyShard commented 1 day ago

Yeah, I continue to hesitate to reorganize the source code as there are still occasional useful patches appearing in the newlib repository.

Yea that makes sense--maybe this could be done upstream?

About tinystdio though, is it also grabbing patches from somewhere else? The compilation rules are fairly complicated for it.

However, removing completely unused code is still a good idea as that doesn't impact importing fixes.

Looking back at my notes, it appears the following files are not used at all; no results in any source files came up in a grep -R "filename" --exclude-dir={.git,build} :

keith-packard commented 1 day ago

Thanks! #860 removes a bunch of these files, while also adding ndbm.c back to the build -- that's a POSIX interface. ftw.c and nftw.c are also POSIX interfaces but are implemented using opendir/readdir, which relies on OS level details we don't generally support, so I removed those too.