On Thu, 27 Jan 2022, trix@xxxxxxxxxx wrote:
> From: Tom Rix <trix@xxxxxxxxxx>
>
> checkpatch reports several hundred formatting errors. Run these files
> through clang-format and knock off some of them.
>
That method seems like a good recipe for endless churn unless checkpatch
and clang-format really agree about these style rules.
Why use checkpatch to assess code style, if we could simply diff the
existing source with the output from clang-format... but it seems that
clang-format harms readability, makes indentation errors and uses
inconsistent style rules. Some examples:
> static unsigned short int max_sectors_per_io = MAX_SECTORS_PER_IO;
> module_param(max_sectors_per_io, ushort, 0);
> -MODULE_PARM_DESC(max_sectors_per_io, "Maximum number of sectors per I/O request (default=MAX_SECTORS_PER_IO=128)");
> -
> +MODULE_PARM_DESC(
> + max_sectors_per_io,
> + "Maximum number of sectors per I/O request (default=MAX_SECTORS_PER_IO=128)");
>
> static unsigned short int max_mbox_busy_wait = MBOX_BUSY_WAIT;
> module_param(max_mbox_busy_wait, ushort, 0);
> -MODULE_PARM_DESC(max_mbox_busy_wait, "Maximum wait for mailbox in microseconds if busy (default=MBOX_BUSY_WAIT=10)");
> +MODULE_PARM_DESC(
> + max_mbox_busy_wait,
> + "Maximum wait for mailbox in microseconds if busy (default=MBOX_BUSY_WAIT=10)");
>
This code is longer for no real improvement.
>
> /*
> * The File Operations structure for the serial/ioctl interface of the driver
> */
> static const struct file_operations megadev_fops = {
> - .owner = THIS_MODULE,
> - .unlocked_ioctl = megadev_unlocked_ioctl,
> - .open = megadev_open,
> - .llseek = noop_llseek,
> + .owner = THIS_MODULE,
> + .unlocked_ioctl = megadev_unlocked_ioctl,
> + .open = megadev_open,
> + .llseek = noop_llseek,
> };
>
> /*
Readability loss.
> - prod_info_dma_handle = dma_map_single(&adapter->dev->dev,
> - (void *)&adapter->product_info,
> - sizeof(mega_product_info),
> - DMA_FROM_DEVICE);
> + prod_info_dma_handle = dma_map_single(
> + &adapter->dev->dev, (void *)&adapter->product_info,
> + sizeof(mega_product_info), DMA_FROM_DEVICE);
>
Note the orphaned first parameter and odd indentation.
>
> static DEF_SCSI_QCMD(megaraid_queue)
>
> -/**
> + /**
> * mega_allocate_scb()
> * @adapter: pointer to our soft state
> * @cmd: scsi command from the mid-layer
Indentation error.
> @@ -418,15 +409,14 @@ static DEF_SCSI_QCMD(megaraid_queue)
> * Allocate a SCB structure. This is the central structure for controller
> * commands.
> */
> -static inline scb_t *
> -mega_allocate_scb(adapter_t *adapter, struct scsi_cmnd *cmd)
> + static inline scb_t *mega_allocate_scb(adapter_t *adapter,
> + struct scsi_cmnd *cmd)
> {
> struct list_head *head = &adapter->free_list;
Same.
> @@ -586,26 +568,25 @@ mega_build_cmd(adapter_t *adapter, struct scsi_cmnd *cmd, int *busy)
>
> ldrv_num = mega_get_ldrv_num(adapter, cmd, channel);
>
> -
> max_ldrv_num = (adapter->flag & BOARD_40LD) ?
> - MAX_LOGICAL_DRIVES_40LD : MAX_LOGICAL_DRIVES_8LD;
> + MAX_LOGICAL_DRIVES_40LD :
> + MAX_LOGICAL_DRIVES_8LD;
>
Churn, if not readability loss. Note the indentation change here is
inconsistent with the indentation change noted above.
> * 6-byte READ(0x08) or WRITE(0x0A) cdb
> */
> - if( cmd->cmd_len == 6 ) {
> - mbox->m_out.numsectors = (u32) cmd->cmnd[4];
> - mbox->m_out.lba =
> - ((u32)cmd->cmnd[1] << 16) |
> - ((u32)cmd->cmnd[2] << 8) |
> - (u32)cmd->cmnd[3];
> + if (cmd->cmd_len == 6) {
> + mbox->m_out.numsectors = (u32)cmd->cmnd[4];
> + mbox->m_out.lba = ((u32)cmd->cmnd[1] << 16) |
> + ((u32)cmd->cmnd[2] << 8) |
> + (u32)cmd->cmnd[3];
>
> mbox->m_out.lba &= 0x1FFFFF;
>
Here, the orphaned term is moved up, next to the =. And yet,
>
> /* Calculate Scatter-Gather info */
> - mbox->m_out.numsgelements = mega_build_sglist(adapter, scb,
> - (u32 *)&mbox->m_out.xferaddr, &seg);
> + mbox->m_out.numsgelements =
> + mega_build_sglist(adapter, scb,
> + (u32 *)&mbox->m_out.xferaddr,
> + &seg);
>
> return scb;
>
... here the first term is moved down and orphaned, which is another
inconsistency.
