Update .clang-format configuration to match linux kernel. #85

tgxn · 2021-03-24T07:36:34Z

tgxn · 2021-03-24T07:41:36Z

Things brings the .clang-format in-line with the linux kernal modules.

zhuyifei1999 · 2021-03-24T19:10:27Z

Uncommenting the # Unknown to clang-format-* ones, formatting again with clang-format version 11.1.0, I get:

$ diff -u xmm7360.c <(clang-format xmm7360.c)
--- xmm7360.c	2021-03-24 14:07:38.011260181 -0500
+++ /dev/fd/63	2021-03-24 14:09:32.750525774 -0500
@@ -409,9 +409,9 @@
 	ring->pages_phys = kzalloc(sizeof(dma_addr_t) * depth, GFP_KERNEL);
 
 	for (i = 0; i < depth; i++) {
-		ring->pages[i] =
-			dma_alloc_coherent(xmm->dev, ring->page_size,
-					   &ring->pages_phys[i], GFP_KERNEL);
+		ring->pages[i] = dma_alloc_coherent(xmm->dev, ring->page_size,
+						    &ring->pages_phys[i],
+						    GFP_KERNEL);
 		ring->tds[i].addr = ring->pages_phys[i];
 	}
 
@@ -730,14 +730,14 @@
 	return -ENOTTY;
 }
 
-static struct file_operations xmm7360_fops = { .read = xmm7360_cdev_read,
-					       .write = xmm7360_cdev_write,
-					       .poll = xmm7360_cdev_poll,
-					       .unlocked_ioctl =
-						       xmm7360_cdev_ioctl,
-					       .open = xmm7360_cdev_open,
-					       .release =
-						       xmm7360_cdev_release };
+static struct file_operations xmm7360_fops = {
+	.read = xmm7360_cdev_read,
+	.write = xmm7360_cdev_write,
+	.poll = xmm7360_cdev_poll,
+	.unlocked_ioctl = xmm7360_cdev_ioctl,
+	.open = xmm7360_cdev_open,
+	.release = xmm7360_cdev_release
+};
 
 static void xmm7360_mux_frame_init(struct xmm_net *xn, struct mux_frame *frame,
 				   int sequence)
@@ -1530,8 +1530,8 @@
 		TTY_DRIVER_REAL_RAW |
 		TTY_DRIVER_DYNAMIC_DEV; // Could this flags be defined in the flags??
 	xmm7360_tty_driver->init_termios = tty_std_termios;
-	xmm7360_tty_driver->init_termios.c_cflag =
-		B115200 | CS8 | CREAD | HUPCL | CLOCAL;
+	xmm7360_tty_driver->init_termios.c_cflag = B115200 | CS8 | CREAD |
+						   HUPCL | CLOCAL;
 	xmm7360_tty_driver->init_termios.c_lflag &= ~ECHO;
 	xmm7360_tty_driver->init_termios.c_ispeed = 115200;
 	xmm7360_tty_driver->init_termios.c_ospeed = 115200;

$ diff -u mux.c <(clang-format mux.c)

tgxn · 2021-03-25T00:35:00Z

I just copied the configuration from here if they have commented lines, won't there be a reason for that?

zhuyifei1999 · 2021-03-25T00:43:05Z

The file was added to mainline around April of 2018 (torvalds/linux@d4ef8d3). At the time llvm/clang 5 was just released (and clang 6 was under development) so I would assume backward compatibility to clang 4 (released Mar 2017) and clang 5 (released Mar 2018) were particularly important.

Thought now that you speak about it, I wonder if asking to get them uncommented in mainline make sense. Hmm...

tgxn · 2021-03-25T01:03:16Z

Yeah that's what I was thinking @zhuyifei1999 ! Why don't they update them to have better rules :')

I see that several other kernel modules seem to "base" their .clang-format on the same file, however some do modify to suit their needs.

I'm happy with whatever approach, but I would err on the side of using the format from the latest tip of kernel. (the one I copied was updated ~28 days ago)

zhuyifei1999 · 2021-03-25T01:16:42Z

I'm happy with whatever approach, but I would err on the side of using the format from the latest tip of kernel. (the one I copied was updated ~28 days ago)

Yeah, that's valid. I just checked and using that of the latest mainline would actually revert the changes:

$ wget https://raw.githubusercontent.com/torvalds/linux/master/.clang-format -O .clang-format -q
$ diff -u xmm7360.c.1 <(clang-format xmm7360.c.1)
--- xmm7360.c.1	2021-03-24 20:12:45.352710914 -0500
+++ /dev/fd/63	2021-03-24 20:13:04.387070093 -0500
@@ -409,9 +409,9 @@
 	ring->pages_phys = kzalloc(sizeof(dma_addr_t) * depth, GFP_KERNEL);
 
 	for (i = 0; i < depth; i++) {
-		ring->pages[i] = dma_alloc_coherent(xmm->dev, ring->page_size,
-						    &ring->pages_phys[i],
-						    GFP_KERNEL);
+		ring->pages[i] =
+			dma_alloc_coherent(xmm->dev, ring->page_size,
+					   &ring->pages_phys[i], GFP_KERNEL);
 		ring->tds[i].addr = ring->pages_phys[i];
 	}
 
@@ -730,14 +730,14 @@
 	return -ENOTTY;
 }
 
-static struct file_operations xmm7360_fops = {
-	.read = xmm7360_cdev_read,
-	.write = xmm7360_cdev_write,
-	.poll = xmm7360_cdev_poll,
-	.unlocked_ioctl = xmm7360_cdev_ioctl,
-	.open = xmm7360_cdev_open,
-	.release = xmm7360_cdev_release
-};
+static struct file_operations xmm7360_fops = { .read = xmm7360_cdev_read,
+					       .write = xmm7360_cdev_write,
+					       .poll = xmm7360_cdev_poll,
+					       .unlocked_ioctl =
+						       xmm7360_cdev_ioctl,
+					       .open = xmm7360_cdev_open,
+					       .release =
+						       xmm7360_cdev_release };
 
 static void xmm7360_mux_frame_init(struct xmm_net *xn, struct mux_frame *frame,
 				   int sequence)
@@ -1530,8 +1530,8 @@
 		TTY_DRIVER_REAL_RAW |
 		TTY_DRIVER_DYNAMIC_DEV; // Could this flags be defined in the flags??
 	xmm7360_tty_driver->init_termios = tty_std_termios;
-	xmm7360_tty_driver->init_termios.c_cflag = B115200 | CS8 | CREAD |
-						   HUPCL | CLOCAL;
+	xmm7360_tty_driver->init_termios.c_cflag =
+		B115200 | CS8 | CREAD | HUPCL | CLOCAL;
 	xmm7360_tty_driver->init_termios.c_lflag &= ~ECHO;
 	xmm7360_tty_driver->init_termios.c_ispeed = 115200;
 	xmm7360_tty_driver->init_termios.c_ospeed = 115200;

I sent an email to Miguel Ojeda (who maintains that file, also CC'ed you) asking if there are plans to uncomment them.

tgxn · 2021-03-25T01:34:41Z

Yep, just noticed your email to Miguel :) Sounds good, will see what happens with that.

Copying it here so I can remember this! 👍

Hi Miguel

I was pushing for an out-of-tree driver I was using to adapt to the
upstream clang-format [1], and it was pointed out how many of its
rules were commented out due to clang-format version reasons. I'm
wondering, since you are the maintainer of that file, are there any
plans to uncomment them, and possibly also adding newly supported
rules since 2018?

I CC'ed Domenic whom I was talking to in the linked issue.

Thanks
YiFei Zhu

[1] https://github.com/xmm7360/xmm7360-pci/pull/85

tgxn · 2021-03-26T02:30:51Z

FWIW:

tgxn@lenovo:/etc/x1_yoga/xmm7360-pci (split_dbus_library) $ clang-format --version
clang-format version 10.0.0-4ubuntu1

annejan

That ForEachMacros though . .

tgxn · 2021-05-03T00:22:03Z

Hey @zhuyifei1999

I noticed the email chain has gone quiet, with Miguel seemingly quite busy. Looks like the minimum for the kernel is clang-format version 10.01 though.

Thinking we upgrade our clang format to be compatible with 10 and merge this in? Then we can always patch from kernel version when they accept/patch those changes?

I'm not 100% sure which lines are compatible/not with v10.01, maybe there's something we can copy already committed somewhere?

zhuyifei1999 · 2021-05-03T01:01:44Z

I'm not 100% sure which lines are compatible/not with v10.01, maybe there's something we can copy already committed somewhere?

I'd suggest just uncomment the "# Unknown to clang-format-something" ones. If upstream would like to add new rules later we can see what they add and adapt.

tgxn · 2021-05-03T01:22:18Z

Okay, I've just uncommented all of the commented lines, since they were all for versions 4.0 and 5.0.

There's still the massive for_each macros, though I think that's fine to leave as-is.

I also would like to add a GitHub Action to perform clang-format and lint, maybe in another PR.

…module_clang_format

Format: update clang-format configuration

7893f39

tgxn mentioned this pull request Mar 24, 2021

C Code Formatting + CI Workflow #78

Merged

format c code with clang-format

ad30f4a

tgxn changed the title ~~Update .clang-format configuration to match linux kernal.~~ Update .clang-format configuration to match linux kernel. Mar 24, 2021

tgxn assigned tgxn and zhuyifei1999 Mar 25, 2021

tgxn added the enhancement New feature or request label Mar 25, 2021

annejan approved these changes Apr 15, 2021

View reviewed changes

Style: Uncomment non-supported rules for clang-format 4.0 and 5.0

00a44ca

Domenic Horner added 2 commits May 11, 2021 08:51

Merge branch 'master' of https://github.com/xmm7360/xmm7360-pci into …

31af209

…module_clang_format

Format c files

0ffedc2

tgxn merged commit 9d9a200 into master May 18, 2021

tgxn deleted the module_clang_format branch May 18, 2021 04:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update .clang-format configuration to match linux kernel. #85

Update .clang-format configuration to match linux kernel. #85

tgxn commented Mar 24, 2021

tgxn commented Mar 24, 2021

zhuyifei1999 commented Mar 24, 2021 •

edited

Loading

tgxn commented Mar 25, 2021

zhuyifei1999 commented Mar 25, 2021

tgxn commented Mar 25, 2021 •

edited

Loading

zhuyifei1999 commented Mar 25, 2021 •

edited

Loading

tgxn commented Mar 25, 2021

tgxn commented Mar 26, 2021

annejan left a comment

tgxn commented May 3, 2021 •

edited

Loading

zhuyifei1999 commented May 3, 2021

tgxn commented May 3, 2021

Update .clang-format configuration to match linux kernel. #85

Update .clang-format configuration to match linux kernel. #85

Conversation

tgxn commented Mar 24, 2021

tgxn commented Mar 24, 2021

zhuyifei1999 commented Mar 24, 2021 • edited Loading

tgxn commented Mar 25, 2021

zhuyifei1999 commented Mar 25, 2021

tgxn commented Mar 25, 2021 • edited Loading

zhuyifei1999 commented Mar 25, 2021 • edited Loading

tgxn commented Mar 25, 2021

tgxn commented Mar 26, 2021

annejan left a comment

Choose a reason for hiding this comment

tgxn commented May 3, 2021 • edited Loading

zhuyifei1999 commented May 3, 2021

tgxn commented May 3, 2021

zhuyifei1999 commented Mar 24, 2021 •

edited

Loading

tgxn commented Mar 25, 2021 •

edited

Loading

zhuyifei1999 commented Mar 25, 2021 •

edited

Loading

tgxn commented May 3, 2021 •

edited

Loading