Figure out how to hookup clang-format

[igaw]

I've experimented with clang-format to reformat ioctl.h. I think it would be possible to use some parts of the reformatting but it got some things pretty wrong. So currently, I don't think we should blindly apply it.

• .clang-format

# SPDX-License-Identifier: GPL-2.0
#
# clang-format configuration file. Intended for clang-format >= 11.
#
# For more information, see:
#
#   Documentation/dev-tools/clang-format.rst
#   https://clang.llvm.org/docs/ClangFormat.html
#   https://clang.llvm.org/docs/ClangFormatStyleOptions.html
#
---
BasedOnStyle: LLVM
Language: Cpp
IndentWidth: 8
UseTab: Always
TabWidth: 8
BreakBeforeBraces: Linux
IndentCaseLabels: false
AlignTrailingComments: true
AllowShortIfStatementsOnASingleLine: false
AllowShortFunctionsOnASingleLine: false
MaxEmptyLinesToKeep: 2

• .clang-format-include

src/nvme/ioctl.h

The above .clang-format above is just my latest version of the configuration while playing with it. So it's likely not really good at all.

[MaisenbacherD]

I agree that we should introduce a .clang-format. We can definitely spend some cycles of applying the formatting and fixing up the .clang-format until we are happy.
Did you try using the .clang-format from the kernel and see if that looks better in your opinion?

[igaw]

I tried the one from the kernel. The main issue is that the enum definition formatting is especially bad for aligned blocks of one-line #defines, aligned designated initializers or aligned enum initializers. Unfortunately, this most of the ioctl.h code falls into one of these categories.

See also https://www.kernel.org/doc/html/next/dev-tools/clang-format.html#missing-support

 struct nvme_uring_cmd {
-       __u8    opcode;
-       __u8    flags;
-       __u16   rsvd1;
-       __u32   nsid;
-       __u32   cdw2;
-       __u32   cdw3;
-       __u64   metadata;
-       __u64   addr;
-       __u32   metadata_len;
-       __u32   data_len;
-       __u32   cdw10;
-       __u32   cdw11;
-       __u32   cdw12;
-       __u32   cdw13;
-       __u32   cdw14;
-       __u32   cdw15;
-       __u32   timeout_ms;
-       __u32   rsvd2;
+       __u8 opcode;
+       __u8 flags;
+       __u16 rsvd1;
+       __u32 nsid;
+       __u32 cdw2;
+       __u32 cdw3;
+       __u64 metadata;
+       __u64 addr;
+       __u32 metadata_len;
+       __u32 data_len;
+       __u32 cdw10;
+       __u32 cdw11;
+       __u32 cdw12;
+       __u32 cdw13;
+       __u32 cdw14;
+       __u32 cdw15;
+       __u32 timeout_ms;
+       __u32 rsvd2;
 };

-#define NVME_IOCTL_ID          _IO('N', 0x40)
-#define NVME_IOCTL_RESET       _IO('N', 0x44)
-#define NVME_IOCTL_SUBSYS_RESET        _IO('N', 0x45)
-#define NVME_IOCTL_RESCAN      _IO('N', 0x46)
-#define NVME_IOCTL_ADMIN_CMD   _IOWR('N', 0x41, struct nvme_passthru_cmd)
-#define NVME_IOCTL_IO_CMD      _IOWR('N', 0x43, struct nvme_passthru_cmd)
-#define NVME_IOCTL_ADMIN64_CMD  _IOWR('N', 0x47, struct nvme_passthru_cmd64)
-#define NVME_IOCTL_IO64_CMD     _IOWR('N', 0x48, struct nvme_passthru_cmd64)
+#define NVME_IOCTL_ID _IO('N', 0x40)
+#define NVME_IOCTL_RESET _IO('N', 0x44)
+#define NVME_IOCTL_SUBSYS_RESET _IO('N', 0x45)
+#define NVME_IOCTL_RESCAN _IO('N', 0x46)
+#define NVME_IOCTL_ADMIN_CMD _IOWR('N', 0x41, struct nvme_passthru_cmd)
+#define NVME_IOCTL_IO_CMD _IOWR('N', 0x43, struct nvme_passthru_cmd)
+#define NVME_IOCTL_ADMIN64_CMD _IOWR('N', 0x47, struct nvme_passthru_cmd64)
+#define NVME_IOCTL_IO64_CMD _IOWR('N', 0x48, struct nvme_passthru_cmd64)

Though for functions the result was pretty okay. The only thing I would like to see is more consistent output:

-static inline int nvme_identify_iocs_ns_csi_user_data_format(nvme_link_t l,
-                       enum nvme_csi csi, __u16 user_data_format, __u8 uidx,
-                       void *data)
+static inline int
+nvme_identify_iocs_ns_csi_user_data_format(nvme_link_t l, enum nvme_csi csi,
+                                          __u16 user_data_format, __u8 uidx,
+                                          void *data)
 {
        return nvme_identify(l, NVME_NSID_NONE, NVME_CNTLID_NONE,
-                            NVME_IDENTIFY_CNS_CSI_NS_USER_DATA_FORMAT,
-                            csi, user_data_format, uidx,
-                            data, NULL);
+                            NVME_IDENTIFY_CNS_CSI_NS_USER_DATA_FORMAT, csi,
+                            user_data_format, uidx, data, NULL);
 }

 /**
@@ -1274,7 +1296,8 @@ static inline int nvme_identify_iocs_ns_csi_user_data_format(nvme_link_t l,
  * Return: 0 on success, the nvme command status if a response was
  * received (see &enum nvme_status_field) or a negative error otherwise.
  */
-static inline int nvme_nvm_identify_ctrl(nvme_link_t l, struct nvme_id_ctrl_nvm *id)
+static inline int nvme_nvm_identify_ctrl(nvme_link_t l,
+                                        struct nvme_id_ctrl_nvm *id)
 {
        return nvme_identify_ctrl_csi(l, NVME_CSI_NVM, id);
 }

E.g. always to put the static inline int on a separate line. I don't like these mixed styles.

If I read the kernel documentation correctly, the suggestion is not to blindly apply the formatting rules.