Re: implicit linkage
On Mon, Oct 13, 2014 at 4:18 PM, Jonathan Dowland <jmtd@debian.org> wrote:
> On Sun, Oct 12, 2014 at 02:48:55PM -0400, Steve Litt wrote:
>> On Sun, 12 Oct 2014 19:02:08 +0100 Martin Read <zen75502@zen.co.uk> wrote:
>> > On 12/10/14 18:13, John Hasler wrote:
>> > > You have no problem with an 1800 line function?
> ...
>> > I have a problem with 1800 line functions in general;
> ...
>> I have no problem with an 1800 line function.
> ...
>
> *What* 1800 line function? The commit URI that was shared was an 1894-line
> *file* with a large function definition starting at line 638 and ending at
> 1890. That's a 1252-line function.
mmm? 1800 vs. 1252 ?
30 years ago, when we still read printouts, 60 lines was considered
the ideal max because that's what would fit on a page.
Nowadays, we use a screen, but 60 lines is hard on the eyes (9 pt or
so), so 40 lines is a good screen-full. But it turns out, with being
about to scroll quickly, that 60 lines is still not hard to reach.
Moreover, 60 lines seems to be a pretty good average for what an
experienced coder can keep in his head.
1800/60 vs. 1252/60 ? 30 times the ideal max vs. 21 times? (Ignoring,
for the sake of your argument, those macros.)
Well, maybe we can look at things from the perspective of new
functionality. New functionality sometimes breaks rules just because
you need to get things in there and going before you can start
figuring out where and how to cut things.
Okay, that repository only goes back to April 2012:
http://cgit.freedesktop.org/systemd/systemd/log/src/core?ofs=1350
at the time of this post. (Give it a month or two, and that link won't
go all the way back anymore.)
The function in question at that point began at line 545 and ended at line 1540.
http://cgit.freedesktop.org/systemd/systemd/tree/src/core/dbus-manager.c?id=b30e2f4c18ad81b04e4314fd191a5d458553773c#n545
That's 996 lines, including the closing brace. Plus-minus one, it's
not going to change much. 16.67 times the ideal max, and, for more
than a year, it just got bigger until some time after a year ago
August. We might assume that non-project people critiquing his code
lit a fire under him.
> Not only that but you're looking at a commit dating from August last year. The
> function doesn't even exist any more in current systemd[1]. There are no
> functions of even a 100 lines length in that file now.
>
> [1] http://cgit.freedesktop.org/systemd/systemd/tree/src/core/dbus-manager.c
A quick scan shows a few over the ideal, but the ideal really is an
ideal target. So it would actually be reasonable now, in terms of
length. If it were not pid 1 code.
At least those macros seem to have been replaced with something less fragile.
>> What I *DO* have a problem with is the guy's welding pam onto his new init,
>> and welding other critical and former separate OS functionalities onto his
>> "toolset", preventing (either technically or by them being removed from the
>> packages) former modules from being used.
>
> Which guy is that? The commit that the URI referenced was written by Lennart
> Poettering, so I guess you mean him; but that commit didn't touch the file that
> was being complained about. Maybe you mean one of the other 17 people who have
> contributed to that file?
You do understand that Steve is simply refusing to keep focused on one
file? (I don't blame him. That one file is not the sum and end of the
problems.)
>> If I were to maintain his code, before reducing the 1800 line function, I'd
>> do something about the function with 20 arguments, with each argument
>> including a function call. I'd replace all of that with a struct pointer.
>
> I'd start with *reading the code* if I were you; something you guys clearly
> aren't doing.
How did he know about that 20 parameter function? And don't forget,
the file in question was in the source, substantially as it was, for
more than a year. How much more, I'll have to find a repository that
goes farther back to find out, but I'm not interested. You want to
look for it for me?
> But if you get past that you'll be pleased to discover that such clean ups and
> refactors are happening quite often.
Now, at any rate.
> See e.g.
> df2d202e6ed4001a21c6512c244acad5d4706c87 ("bus: let's simplify things by
> getting rid of unnecessary bus parameters"). I'll leave you to guess the author
> of that one.
Well, I've done a little mousing around in the repository (current, as
well as historical) and it looks like that particular file is part of
the pid 1 code. Correct me if I'm wrong.
Even conceptually, pid 1 code should not be managing dbus. Too much
can go wrong, too many opportunities to get pid 1 chasing it's tail
trying to parse an error state. And the code in that file, much
improved as it is in current, looks like code that can get into
exactly that kind of trouble.
I'd have to dig way down deep to be positive, but I'm still extremely
unimpressed.
Get pid 1 down to 100 lines of C, no loops, no functions called, then
I'll be impressed.
Heh. No, that's talking about getting Linux ready to compete seriously
in embedded, and the kernel needs a bit of work before we start
talking about that. (Not that you can't use it now, sans systemd, in
some of the less demanding embedded applications. There are some
embedded applications it will work for, with reasonable pid 1
processes.)
Setting aside initialization code, pid 1 should target less than 1000
lines of C in the main loop. (If we were to use dash or other
streamlined shells, we might set a target of 100 lines of code.) Loops
and subroutines should be carefully metered for maximum execution
paths, and proven to be deterministic, with a maximum execution path
of less than 500 lines of C.
(I think this target would be significantly better than sysv-init, but
you have to set a really good target to get things into a reasonable
range.)
It should not interface directly with dbus, cgroups, udev, or pamd. It
should delegate interfacing to such things to non-pid 1 daemons, and
restrict itself strictly to keeping the daemons themselves working
together.
There are other goals I should mention, but I don't imagine Lennart
Poettering is even the slightest interested in what we have to say
here, so I won't bother.
No matter how much better it is now than last year, the code is still
just plain wrong.
--
Joel Rees
Be careful when you see conspiracy.
Look first in your own heart,
and ask yourself if you are not your own worst enemy.
Arm yourself with knowledge of yourself.
Reply to: