readlink bugs

[Denis Vlasenko]

On 10/20/06, Bernhard Fischer <rep.nop at aon.at> wrote:
> On Fri, Oct 20, 2006 at 10:13:31AM +0200, Bernhard Fischer wrote:
> >On Thu, Oct 19, 2006 at 07:30:56PM +0200, Denis Vlasenko wrote:
> >>On Wednesday 18 October 2006 07:41, Alexander Griesser wrote:
> >
> >>Bingo! static busybox doesn't fflush stdout!
> >>This workaround made readlink work:
> >
> >No! :)

I am not going to apply it.

> >>int readlink_main(int argc, char **argv)
> >>{
> >>        char *buf;
> >>        unsigned opt = ENABLE_FEATURE_READLINK_FOLLOW ?
> >>                        getopt32(argc, argv, "f") : 0;
> >>
> >>        if (argc != (ENABLE_FEATURE_READLINK_FOLLOW ? optind + 1 : 2))
> >>                        bb_show_usage();
> >>
> >>        if (opt & READLINK_FLAG_f)
> >>                buf = realpath(argv[optind], bb_common_bufsiz1);
> >>        else
> >>                buf = xreadlink(argv[ENABLE_FEATURE_READLINK_FOLLOW ? optind : 1]);
> >>
> >>        if (!buf)
> >>                return EXIT_FAILURE;
> >>        puts(buf);
> >->+       fflush(stdout);
> >>
> >>        if (ENABLE_FEATURE_CLEAN_UP && buf != bb_common_bufsiz1)
> >>                free(buf);
> >>
> >->        return EXIT_SUCCESS;
> >+      bb_fflush_stdout_and_exit(EXIT_SUCCESS);
> >>}
> >>
> >>Need to take a look how glibc managed to botch it...
> >
> >Please don't botch busybox but reuse the stuff that is already there ;)

I won't. That was just how one can prove that glibc doesn't flush streams.

> >   text    data     bss     dec     hex filename
> >     98       0       0      98      62 debianutils/readlink.o.orig
> >    110       0       0     110      6e debianutils/readlink.o.vda
> >    103       0       0     103      67 debianutils/readlink.o.bb_fflush
> >
> >cheers,
>
> hm. Please see the questions in the attached patch that results in
> shrinking it back to under 100 bytes.
>
>    text    data     bss     dec     hex filename
>      94       0       0      94      5e debianutils/readlink.o.attached
>
> Attached patchlet also contains an extension to getopt32's
> opt_complementary; =N to check for exactly N arguments.
> Rational: Several applets do something to the effect of
> (if argc != mynumber)
>         bb_show_usage();
> While it seems that saying opt_complementary="-1?1" for mynumber=1
> works, it doubles the size as compared to the proposed "=1" string.
> Needless to say that the impl of that =N support in getopt32.c that i
> have attached is surprisingly big, but i didn't look closely on how to
> keep it down to a reasonable size increase:
> $ size libbb/getopt32.o*
>    text    data     bss     dec     hex filename
>     979       4       0     983     3d7 libbb/getopt32.o.oorig
>    1014       4       0    1018     3fa libbb/getopt32.o
>
> Is this =N support something that you consider useful?

Well, why not? Will likely apply when I will get back home.
--
vda