Page MenuHomeFreeBSD

style(9): deprecate blank lines without local vars
ClosedPublic

Authored by brooks on Dec 3 2024, 7:22 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Jan 4, 8:45 AM
Unknown Object (File)
Thu, Dec 12, 6:48 PM
Unknown Object (File)
Dec 6 2024, 6:34 AM
Unknown Object (File)
Dec 4 2024, 9:44 PM

Details

Summary

Requiring a blank line at the top of a function if there are no local
declerations dates to the original style.9 commit (b030a30523649) and was
present in the first version of admin/style/style from mckusick@ in 1991.
It's certainly consistant to have a blank line after a null-set of
variables, but today it's an oddity unique to BSD source code.

Document it as a historic practice and encourage removal in the context
of other changes, but not sweeping commits.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

brooks requested review of this revision.Dec 3 2024, 7:22 PM

I thought we'd already went this far

This revision is now accepted and ready to land.Dec 3 2024, 7:24 PM
emaste added a subscriber: emaste.

I thought we'd already went this far

Nope, we only went as far as downgrading a required blank line to optional.

I thought we'd already went this far

The only prior public mentions I could find are D20448 and D43293. The first involved several people and doesn't hint at such an intent at all. The second involves only one people, so didn't hint at a consensus.

Without context it seems like foolish consistency.

Visual consistency is important to some, so I wouldn't call it like that though I see what you mean.

(snip) but not sweeping commits.

I think sweeping commits that are standalone could be a problem, whereas those that are part of a series of significant changes in the same file are usually not (since there are lots of changes around anyway). And sweeping commits are better in maintaining visual consistency.

Personally, I've never seen the convention of a blank line at start of functions without local variables in other places than *BSD. At the same time, it is still in widespread use here. All in all, I am agnostic to this change.

I'm particularly glad that @brooks put this revision up. Even if this change might be considered a minor point of style(9), it is typically the kind of change for which it is great that discussion systematically happens in the open, and for several reasons: to ensure that there is an actual consensus (sometimes noting that there isn't), to integrate feedback and, once settled, to help spread the word.

share/man/man9/style.9
825

FWIW the blank line advice dates to the first version of admin/style/style from @mckusick on May 20, 1991, which had:

f3()
{
					/* Empty line if no variables. */
	return (1);
}

For what it's worth, we do discuss these things in public all the time. I don't appreciate the finger wagging over it. We've often had a quick discussion on irc or a developer summit followed by a quick email to arch@. The fact that many thought it obsolete suggests at least the first part happened. It's also an overreaction to a small breach in a protocol that typically happens.

It used to impossible to change style(9) and it's taken years to get it moving again. The process usually is open to a fault. Please keep that in mind if one minor change was not super open at every step. The right thing isn't to cite history and berate us, but rather to suggest a cc to arch@ if you don't think it's been public enough. I agree that we should circulate these changes wide, where appropriate, but just suggest we do that in the future.

I've tempered the commit message text.

I really do wonder why this came about given the unix heritage of teletype terminals where vertical space was at a premium... I suppose the ability to find the first line of code by searching for ^$ might have been a motivation.

In D47887#1092313, @imp wrote:

For what it's worth, we do discuss these things in public all the time.

That is not the impression I had in this particular case based on your reaction in D47618 and my knowledge of style(9), and I was worried about that. That's also why I searched in some of the history I have access to, to see what I could have missed.

We've often had a quick discussion on irc or a developer summit followed by a quick email to arch@. The fact that many thought it obsolete suggests at least the first part happened.

AFAIK, there is no IRC archive, so I don't think it's reasonable to expect that new members can be aware of everything said there, all the more with the amount of informal chat happening. As you admit it, there was no email to arch@, which by contrast is both fully public and archived. And of course there was the plain contradictory statement in style(9), which seems the most appropriate document for such matters.

It's also an overreaction to a small breach in a protocol that typically happens.

What brooks@ initially said was in clear contradiction with the content of style(9), and you then intervened with a comment that in part was basically saying "you should have just done it and trust us". Call my response "overreaction" if you want. I don't think having doubts after such a start was an unreasonable reaction.

I don't appreciate the finger wagging over it.

There is no finger wagging here, only me worried and trying to come up with constructive comments (and I'm not even the one who created this revision, just the messenger showing the contradiction).

The right thing isn't to cite history and berate us, but rather to suggest a cc to arch@ if you don't think it's been public enough.

Concerning history, I'm just citing facts (the ones I could find). I don't know who is this "us" you're referring to, but be it you personally, srcmgr@, the committers or the community at large, I certainly don't berate "you" (quite the opposite, actually). This is completely uncalled-for. If you were searching for "overreaction", there you have it.

I'm sorry to have to formulate this, but could you please stop this borderline attitude of yours of putting words in my mouth or trying to second guess my feelings? It is not going to get us anywhere pleasant nor constructive.

I agree that we should circulate these changes wide, where appropriate, but just suggest we do that in the future.

That was actually my main intent when writing "it is typically the kind of change for which it is great that discussion systematically happens in the open", i.e., encouraging that for the future, and I wish it had been received essentially like that.

But I'm still finding some relief from seeing that we at least seem to agree on the most important point.

In D47887#1092313, @imp wrote:

For what it's worth, we do discuss these things in public all the time.

That is not the impression I had in this particular case based on your reaction in D47618 and my knowledge of style(9), and I was worried about that. That's also why I searched in some of the history I have access to, to see what I could have missed.

We've often had a quick discussion on irc or a developer summit followed by a quick email to arch@. The fact that many thought it obsolete suggests at least the first part happened.

AFAIK, there is no IRC archive, so I don't think it's reasonable to expect that new members can be aware of everything said there, all the more with the amount of informal chat happening. As you admit it, there was no email to arch@, which by contrast is both fully public and archived. And of course there was the plain contradictory statement in style(9), which seems the most appropriate document for such matters.

Yes. A the same time, though, new members have to accept that our culture does have a large element of collective experience, some of which isn't well documented though. You'll not find perfect (or even imperfect) records of this. The discussions, though, do affect the collective memory and expectations of the group. Since this is a volunteer organization, there's not always time to write it all down. I do agree that it would be better if we had such records and that style(9) was always a perfect reflection of the current stands. But it will fall short at times, and the best we can do is to document the omissions when it becomes apparent.

It's also an overreaction to a small breach in a protocol that typically happens.

What brooks@ initially said was in clear contradiction with the content of style(9), and you then intervened with a comment that in part was basically saying "you should have just done it and trust us". Call my response "overreaction" if you want. I don't think having doubts after such a start was an unreasonable reaction.

I don't appreciate the finger wagging over it.

There is no finger wagging here, only me worried and trying to come up with constructive comments (and I'm not even the one who created this revision, just the messenger showing the contradiction).

Yea. This was a bit of a hair-trigger reaction on my part, which I now see. My apologies. As I explain below, I thought you were advocating for something different than what it's clear to me you are suggesting. It was based on that which caused my reaction. I see now it was unfair and unduly influenced by battles of the past....

The right thing isn't to cite history and berate us, but rather to suggest a cc to arch@ if you don't think it's been public enough.

Concerning history, I'm just citing facts (the ones I could find). I don't know who is this "us" you're referring to, but be it you personally, srcmgr@, the committers or the community at large, I certainly don't berate "you" (quite the opposite, actually). This is completely uncalled-for. If you were searching for "overreaction", there you have it.

I'm sorry to have to formulate this, but could you please stop this borderline attitude of yours of putting words in my mouth or trying to second guess my feelings? It is not going to get us anywhere pleasant nor constructive.

I didn't think I was doing that, and so this is valuable feedback. Please accept my apologies for reading your messages through a lens that ultimately was unfair to what you were trying to accomplish. It's clear you're not familiar with the battles of the past around style(9) and it's unfair of me to read your message though that lens.

I agree that we should circulate these changes wide, where appropriate, but just suggest we do that in the future.

That was actually my main intent when writing "it is typically the kind of change for which it is great that discussion systematically happens in the open", i.e., encouraging that for the future, and I wish it had been received essentially like that.

Yea. I misread it based on far too many battles in the past: it biased me to reading it as a blanket complaint about all these things and an effort to change my mind about needing an open process (which were battles 10 or 15 years ago, now, when any attempt to change style(9) was impossible). I already agree with that and worked for years to move things from severely ossified standard to one we could change and revise.

But I'm still finding some relief from seeing that we at least seem to agree on the most important point.

Yes. We do agree on the main point.

So please accept my apologies for being too grumpy with you. I've explained why, but that still doesn't excuse it and I'll try to do better in the future.

So looking at TUHS:

The blank line isn't present in Research Unix, except maybe accidentally.
The blank lines isn't present too much in 3BSD and 4BSD.
By 4.2BSD it's present in new code (UFS, TCP/IP)

And looking at other sources:

Commercial AT&T-based unix only has this from BSD-derived code
SunOS 4 doesn't have them, except in the lightly-touched BSD code

So this appears to be a Berkeley thing from the 80s. Why? I don't know....

So anyway, that's the ancient history behind where this came from... It also appears to be unique to BSD Unix, but that's just a recollection from the discussions in the past, I didn't go hunt down data to support that. IIRC, people offered X11 and the gnu software as an example of places where the blank line wasn't done, except occasionally.

In D47887#1092651, @imp wrote:

Yes. A the same time, though, new members have to accept that our culture does have a large element of collective experience (...)

I understand (and accept) that, and will see how to lower the possible frictions there.

(...) the best we can do is to document the omissions when it becomes apparent.

Yes. This was one main part of my request (I have already evoked the other in the previous message).

So please accept my apologies for being too grumpy with you. I've explained why, but that still doesn't excuse it and I'll try to do better in the future.

I see. I accept these apologies, and much appreciate them and your efforts to self-examination and realization. Thanks!

Sorry, late to comment.

Adding a bit of history from Keith Bostic:

From: Keith Bostic <keith@bostic.com>
Date: Wed, 18 Dec 2024 11:31:02 -0800
Subject: Re: CSRG C Style Document
To: Kirk McKusick <mckusick@mckusick.com>

From: Kirk McKusick <mckusick@mckusick.com>
Date: Tue, 17 Dec 2024 22:19:49 -0800
To: keith@bostic.com (Keith Bostic)
Subject: CSRG C Style Document

There has been a C-style discussion going on in FreeBSD, see:

https://reviews.freebsd.org/D47887

Looking back through the SCCS logs for the CSRG style guide, I found
that you were the one that added the style guideline that functions
with no local variables should have a blank line after the functions's
opening brace. My best guess is that you were just documenting what we
had been doing in our code. But wonder if you recall your motivation.

~Kirk

OK, that was a fun dive. :)

In short, I don't recall specific motivations, my guess is that for
whatever reason, the first version I created had that rule, probably taken
from whatever files I was looking through at the time.

In long, as far as I can tell:

May 18, 1991, Keith Muller asked me for a style guide for code he was
working on, and I created one from scratch. I sent email that day:

From bostic Sat May 18 00:14:23 1991
To: donn, karels, marc, mckusick, sklower, william
Subject: style module
Status: O

I just got asked for a "style" example for the Nth time -- so I made
one up, it's on okeeffe:~bostic/frag/style. It'd probably be good if
everybody took a quick pass.

--keith

I don't see any copies of that file around, but since I sent a style guide
to Keith Muller 20 minutes later, I'm guessing there are minimal if any
differences between the two. And, that first copy I sent him has this:

f3()
{

/* Empty line if no variables. */

return (1);

}

It looks like you committed that file into SCCS on May 20th (I didn't diff
them, but it looks right), and then did your first pass over it. After you
committed your changes, I did another pass, removing the f3() function and
moving the comment about empty lines to the usage() function, where it
ended up.

--keith