Re: [PATCH] linux-user/flatload.c: Remove unused bFLT shared-library and ZFLAT code

[Philippe Mathieu-Daudé]

On 11/4/24 13:53, Peter Maydell wrote:
> Ever since the bFLT format support was added in 2006, there has been
> a chunk of code in the file guarded by CONFIG_BINFMT_SHARED_FLAT
> which is supposedly for shared library support.  This is not enabled
> and it's not possible to enable it, because if you do you'll run into
> the "#error needs checking" in the calc_reloc() function.
>
> Similarly, CONFIG_BINFMT_ZFLAT exists but can't be enabled because of
> an "#error code needs checking" in load_flat_file().
>
> This code is obviously unfinished and has never been used; nobody in
> the intervening 18 years has complained about this or fixed it, so
> just delete the dead code.  If anybody ever wants the feature they
> can always pull it out of git, or (perhaps better) write it from
> scratch based on the current Linux bFLT loader rather than the one of
> 18 years ago.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   linux-user/flat.h     |   5 +-
>   linux-user/flatload.c | 293 ++----------------------------------------
>   2 files changed, 11 insertions(+), 287 deletions(-)


> @@ -268,40 +115,7 @@ calc_reloc(abi_ulong r, struct lib_info *p, int curid, int 
> internalp)
>       abi_ulong text_len;
>       abi_ulong start_code;
>   
> -#ifdef CONFIG_BINFMT_SHARED_FLAT
> -#error needs checking
> -    if (r == 0)
> -        id = curid;    /* Relocs of 0 are always self referring */
> -    else {
> -        id = (r >> 24) & 0xff;       /* Find ID for this reloc */
> -        r &= 0x00ffffff;   /* Trim ID off here */
> -    }
> -    if (id >= MAX_SHARED_LIBS) {
> -        fprintf(stderr, "BINFMT_FLAT: reference 0x%x to shared library %d\n",
> -                (unsigned) r, id);
> -        goto failed;
> -    }
> -    if (curid != id) {
> -        if (internalp) {
> -            fprintf(stderr, "BINFMT_FLAT: reloc address 0x%x not "
> -                    "in same module (%d != %d)\n",
> -                    (unsigned) r, curid, id);
> -            goto failed;
> -        } else if (!p[id].loaded && is_error(load_flat_shared_library(id, p))) 
> {
> -            fprintf(stderr, "BINFMT_FLAT: failed to load library %d\n", id);
> -            goto failed;
> -        }
> -        /* Check versioning information (i.e. time stamps) */
> -        if (p[id].build_date && p[curid].build_date
> -            && p[curid].build_date < p[id].build_date) {
> -            fprintf(stderr, "BINFMT_FLAT: library %d is younger than %d\n",
> -                    id, curid);
> -            goto failed;
> -        }
> -    }
> -#else
>       id = 0;

I note 'curid' is not used, and 'id' is always 0, because

#define    MAX_SHARED_LIBS                 (1)

Having:

struct lib_info libinfo[MAX_SHARED_LIBS];

Maybe we can remove MAX_SHARED_LIBS entirely to simplify
further?

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

> -#endif
>   
>       start_brk = p[id].start_brk;
>       start_data = p[id].start_data;
> @@ -425,12 +239,10 @@ static int load_flat_file(struct linux_binprm * bprm,
>       if (rev == OLD_FLAT_VERSION && flat_old_ram_flag(flags))
>           flags = FLAT_FLAG_RAM;
>   
> -#ifndef CONFIG_BINFMT_ZFLAT
>       if (flags & (FLAT_FLAG_GZIP|FLAT_FLAG_GZDATA)) {
> -        fprintf(stderr, "Support for ZFLAT executables is not enabled\n");
> +        fprintf(stderr, "ZFLAT executables are not supported\n");
>           return -ENOEXEC;
>       }
> -#endif