clang-format libalias with D30259 applied

[emaste]

Summary

For reference, to examine the differences

@donner is fixing style in libalias, in D30259. I applied the patch from D30259 and then ran sys/netinet/libalias/* through clang-format to see if it finds additional issues and continue the evaluation of clang-format's suitability for automated code formatting.

[emaste]

@donner for context I have been looking at clang-format's output on some sample files, e.g. D26340, and @domagoj.stolfa_gmail.com did the same with uncrustify in D30248.

[donner]

Thank you for the cool idea! There are some interesting ideas which are worth to be applied.

I don't think that "tabs for aligment" to ease reading large lists of functions should be removed.

[donner]

Thank you for the input.

[emaste]

Regen after effc8e57fbf03d7f1423c662caf2b63f0d1d21a9

[imp]

I read through about half of these. But there's a number of trends:
alined things are messed up, and introduces a lot of churn
the wrapping to make things optimally introduces a lot of churn: some better, some worse, some just different.
single line comments are messed up in the wrapping and at other times
space after queue macros is wrong
space after 'new' is wrong
Slavish enforcement of 80 columns often makes the code a bit harder to read.

On the plus side, the include sorting looked good.

sys/netinet/libalias/alias.c

143

The above shuffling of includes is fine.

320

this is gratuitous in that it was fine before . In this case, I don't think it matters much, but there's many places people put logically similar things together on one line and break with the strictly optimal 80 column packing.

361

These, on the other hand are generally more readable, but I'd also be inclined to say that the last couple are fine either way and relaxing 80 columns would result in more readable code.

883

this shouldn't be snugged up a line. The line should end with {

889

this is wrong } belongs on the next line.

1280

The aligned comments is better, imho.

1440

this is ugly and not style(9) compliant

sys/netinet/libalias/alias.h

40

It's unfortunate we can't say both tabs and spaces are good there...

86–94

I really don't like this. But this issue is also present in the gratuitous reformatting of variable declarations.

151

I find the reformatted stuff harder to read. the original columns were better.

sys/netinet/libalias/alias_db.c

90

I think this is gratuitous and inconsistent with how we do things elsewhere, while the old code was consistent.

160

this is a bad choice.

170

all this wrapping makes it harder to read

219

not style(9) compliant

422–423

not consistent with existing practice.

sys/netinet/libalias/alias_util.c

160

this is wrong.

[emaste]

I read through about half of these. But there's a number of trends:
alined things are messed up, and introduces a lot of churn
...

Indeed - my main point isn't that we should run clang-format over the whole tree (at least in the near future), but rather if someone submitted a new file or a patch formatted like this, would it be fine. This directory provided a good sample as it was undergoing style improvements anyway. My hope is that we can eliminate the things clang-format just gets completely wrong.

sys/netinet/libalias/alias.c

320

I agree the churn for this sort of thing is not worth it in isolation.

361

but also I think improved readability here is just a side-effect of rewrapping,

883

yes, a little clang-format improvement ought to be made here

1440

the old version wasn't either :)

my guess is it won't be possible to teach clang-format that a /* single line comment */ should be rewrapped as
/*

• a comment like this */

and that we'd just have to fix this manually first. (I assume it would have been left alone if the comment fit in 80 cols, although putting the 1st and 2nd arg on one line probably also can't be fixed)

sys/netinet/libalias/alias.h

86–94

Yeah. If we start going farther with clang-format I would probably suggest we just /* clang-format off */ around these sorts of things

sys/netinet/libalias/alias_db.c

160

Yeah, (la->sctpLinkCount >> 1) + seems better, although is it over 80 cols with the comment? But either way it seems hard to read before or after

219

Yes same case of rewrapping comment over 80 cols. Although, bikesheds be damned, I wish we used

/* This sort of style
 * for multiline comments
 */

since I think it is just as readable and saves an extra line

sys/netinet/libalias/alias_util.c

160

@arichardson clang-format treating this as a c++ file?

[arichardson]

sys/netinet/libalias/alias_util.c

160

This definitely looks like a bug we should report upstream.

[imp]

In D30260#681726, @emaste wrote:

I read through about half of these. But there's a number of trends:
alined things are messed up, and introduces a lot of churn
...

Indeed - my main point isn't that we should run clang-format over the whole tree (at least in the near future), but rather if someone submitted a new file or a patch formatted like this, would it be fine. This directory provided a good sample as it was undergoing style improvements anyway. My hope is that we can eliminate the things clang-format just gets completely wrong.

Yea. That's why I so picky. My comments about the churn is because git clang-format would gratuitously reformat the patches and make the style of the file mixed... I understand that this is a science experiment to see what can be used.

[emaste]

regen with clang-format17 and D29870 applied