[Toybox] [PATCH] net.c: add missing static.

enh enh at google.com
Mon Jul 15 08:48:43 PDT 2019


On Sat, Jul 13, 2019 at 4:23 PM Rob Landley <rob at landley.net> wrote:
>
> On 7/12/19 5:52 PM, enh via Toybox wrote:
> > Pointed out during code review of the recent refactor.
>
> Applied, although _mostly_ I don't do static functions in lib, and this is the
> only one in lib/net.c.
>
> Commands shouldn't export unnecessary functions, because if anything else uses
> them it should go into lib. (There's nowhere to put the function declarations
> other than command_main(), and there's nasty "is this command enabled yes/no"
> potential link errors if you just reach across toys/*/*.c files like that.)
>
> But the _point_ of lib/*.c is to export shared infrastructure. What does static
> really accomplish there? lib/deflate.c is full of them because it started life
> as a command. Almost all the rest of the statics in lib/ are variables, not
> functions...
>
> I see you're saying "don't use this interface, use the other interface" and
> trying to hide implementation details. (Except they're not hidden, are they?
> They're right there...?)

yeah, but when you're searching through the file for something you
think is probably there but don't know the name of, `static` is a nice
clear "next, please". hopefully the fact that xbind and xconnect are
now just "x" versions of bind and connect should mean this isn't
something anyone has to search for again though :-)

> Sigh. Somebody checking a bad patch into their own fork of the project isn't
> something this sort of thing would actually prevent, but oh well. If you're
> doing that lib/args.c parse_optflaglist() should probably have one. (It's more
> or less inlined in the function...)
>
> Rob
>
> (I've been squinting at lib/args.c because shells have a "getopts" that I've
> never used but apparently it's a thing and I should support it and I'm wondering
> if any of the code I've got is reusable for this...)

/me looks, finds it's different to getopt(1) [which is needed by AOSP
and on my long-term to-do list, but luckily seems like a thin wrapper
around getopt_long(3) anyway], bangs forehead on table...



More information about the Toybox mailing list