Add a basic clang-format configuration file

[arichardson]

Summary

This gets reasonably close to the existing format in sys/kern but will
probably require some changes to upstream clang-format before it can be
used as the default formatting tool.

Test Plan

I tried formatting a few files in sys/kern and the result is pretty close to
the existing code. However, I'm not sure what a good "gold standard" source
file would be.

[emaste]

Perhaps we should put a comment at the beginning of this file explaining that it's not a perfect match for style(9) and providing an example or two of how it can be used?

I had a much less comprehensive .clang-format and used it like so:

clang-format file.c > formatted.c
kdiff3 file.c formatted.c &

and then just edited file.c to manually apply appropriate changes.

The .clang-format here is much better than what I had and gets quite close. Some changes it made and some comments on those changes below, but my overall comment: this is indeed already useful as is for work I have in progress.

 struct glyph {
-       TAILQ_ENTRY(glyph)       g_list;
-       SLIST_ENTRY(glyph)       g_hash;
-       uint8_t                 *g_data;
-       unsigned int             g_index;
+       TAILQ_ENTRY(glyph) g_list;
+       SLIST_ENTRY(glyph) g_hash;
+       uint8_t *g_data;
+       unsigned int g_index;
 };

We have a mix of lining up and not lining up variable names in the tree; examples in style(9) have them lined up.

{
 
        (void)fprintf(stderr,
-"usage: vtfontcvt [-w width] [-h height] [-v] normal.bdf [bold.bdf] out.fnt\n");
+           "usage: vtfontcvt [-w width] [-h height] [-v] normal.bdf [bold.bdf] out.fnt\n");
        exit(1);
 }

Existing code let 80-cols take priority over indentation. Not something we should try to address in the tool, if we really need something like this it would have to be handled as an exception.

        hash = fnv_32_buf(bytes, wbytes * height, FNV1_32_INIT) % FONTCVT_NHASH;
-       SLIST_FOREACH(gl, &glyph_hash[hash], g_hash) {
+       SLIST_FOREACH (gl, &glyph_hash[hash], g_hash) {
                if (memcmp(gl->g_data, bytes, wbytes * height) == 0) {

Inserted a space after the FOREACH macro, existing code in the tree generally has no space in FOREACH(. But we have space in for ( so clang-format's take is probably correct here.

                        if (bbw < 1 || bbh < 1 || bbw > fbbw || bbh > fbbh ||
                            bbox < fbbox || bboy < fbboy)
-                               errx(1, "Broken bitmap with "
+                               errx(1,
+                                   "Broken bitmap with "
                                    "BBX %d %d %d %d at line %u",
                                    bbw, bbh, bbox, bboy, linenum);
                        bbwbytes = howmany(bbw, 8);

I'm not really sure what happened here.

 static int
-parse_bitmap_line(uint8_t *left, uint8_t *right, unsigned int line,
-    unsigned int dwidth)
+parse_bitmap_line(
+    uint8_t *left, uint8_t *right, unsigned int line, unsigned int dwidth)
 {

-                       snprintf(fmt_str, sizeof(fmt_str), "%%%ux",
-                           chars_per_row);
+                       snprintf(
+                           fmt_str, sizeof(fmt_str), "%%%ux", chars_per_row);

These two cases, c-f prefers to wrap before all arguments if they then fit in one line.

                        if (bytes != NULL)
-                               errx(1, "malformed input: Height tag after font data");
+                               errx(1,
+                                   "malformed input: Height tag after font data");
                        set_height(atoi(ln + 10));

Even with wrapping the string is still over 80 cols, so might as well leave it alone.

-       if (write_glyphs(fp) != 0 ||
-           write_mappings(fp, VFNT_MAP_NORMAL) != 0 ||
+       if (write_glyphs(fp) != 0 || write_mappings(fp, VFNT_MAP_NORMAL) != 0 ||
            write_mappings(fp, VFNT_MAP_NORMAL_RH) != 0 ||
            write_mappings(fp, VFNT_MAP_BOLD) != 0 ||
            write_mappings(fp, VFNT_MAP_BOLD_RH) != 0) {

If we really want to keep cases like this they'd be done as an exception.

[arichardson]

In D20533#443680, @emaste wrote:

Perhaps we should put a comment at the beginning of this file explaining that it's not a perfect match for style(9) and providing an example or two of how it can be used?

I had a much less comprehensive .clang-format and used it like so:

clang-format file.c > formatted.c
kdiff3 file.c formatted.c &

and then just edited file.c to manually apply appropriate changes.

The .clang-format here is much better than what I had and gets quite close. Some changes it made and some comments on those changes below, but my overall comment: this is indeed already useful as is for work I have in progress.

 struct glyph {
-       TAILQ_ENTRY(glyph)       g_list;
-       SLIST_ENTRY(glyph)       g_hash;
-       uint8_t                 *g_data;
-       unsigned int             g_index;
+       TAILQ_ENTRY(glyph) g_list;
+       SLIST_ENTRY(glyph) g_hash;
+       uint8_t *g_data;
+       unsigned int g_index;
 };

We have a mix of lining up and not lining up variable names in the tree; examples in style(9) have them lined up.

Do we want all variables or just struct members to line up? If so we can set AlignConsecutiveAssignments: true.

{
 
        (void)fprintf(stderr,
-"usage: vtfontcvt [-w width] [-h height] [-v] normal.bdf [bold.bdf] out.fnt\n");
+           "usage: vtfontcvt [-w width] [-h height] [-v] normal.bdf [bold.bdf] out.fnt\n");
        exit(1);
 }

Existing code let 80-cols take priority over indentation. Not something we should try to address in the tool, if we really need something like this it would have to be handled as an exception.

Yeah I'm not sure there is a good solution here other than adding /* clang-format off */ and /* clang-format on */. However, if you use git-clang-format that will only modify changed lines so those should not change.

        hash = fnv_32_buf(bytes, wbytes * height, FNV1_32_INIT) % FONTCVT_NHASH;
-       SLIST_FOREACH(gl, &glyph_hash[hash], g_hash) {
+       SLIST_FOREACH (gl, &glyph_hash[hash], g_hash) {
                if (memcmp(gl->g_data, bytes, wbytes * height) == 0) {

Inserted a space after the FOREACH macro, existing code in the tree generally has no space in FOREACH(. But we have space in for ( so clang-format's take is probably correct here.

                        if (bbw < 1 || bbh < 1 || bbw > fbbw || bbh > fbbh ||
                            bbox < fbbox || bboy < fbboy)
-                               errx(1, "Broken bitmap with "
+                               errx(1,
+                                   "Broken bitmap with "
                                    "BBX %d %d %d %d at line %u",
                                    bbw, bbh, bbox, bboy, linenum);
                        bbwbytes = howmany(bbw, 8);

I'm not really sure what happened here.

Yeah I've seen some odd line breaks related to string constants as well. I guess we need to file an LLVM bug for that.

 static int
-parse_bitmap_line(uint8_t *left, uint8_t *right, unsigned int line,
-    unsigned int dwidth)
+parse_bitmap_line(
+    uint8_t *left, uint8_t *right, unsigned int line, unsigned int dwidth)
 {

-                       snprintf(fmt_str, sizeof(fmt_str), "%%%ux",
-                           chars_per_row);
+                       snprintf(
+                           fmt_str, sizeof(fmt_str), "%%%ux", chars_per_row);

These two cases, c-f prefers to wrap before all arguments if they then fit in one line.

                        if (bytes != NULL)
-                               errx(1, "malformed input: Height tag after font data");
+                               errx(1,
+                                   "malformed input: Height tag after font data");
                        set_height(atoi(ln + 10));

Even with wrapping the string is still over 80 cols, so might as well leave it alone.

We could turn on the BreakStringLiterals option to fix this but I'm not sure it will do the right thing.

-       if (write_glyphs(fp) != 0 ||
-           write_mappings(fp, VFNT_MAP_NORMAL) != 0 ||
+       if (write_glyphs(fp) != 0 || write_mappings(fp, VFNT_MAP_NORMAL) != 0 ||
            write_mappings(fp, VFNT_MAP_NORMAL_RH) != 0 ||
            write_mappings(fp, VFNT_MAP_BOLD) != 0 ||
            write_mappings(fp, VFNT_MAP_BOLD_RH) != 0) {

If we really want to keep cases like this they'd be done as an exception.

[pfg]

Thanks for working on this. For the record,

NetBSD has a GSoC project to improve the clang-format tool with more BSD-friendly constructs:

https://summerofcode.withgoogle.com/projects/#5661503443697664