Josh Holland <jrh@xxxxxxxxxxx> writes:
> This is a patch to the rar_driver.c and rar_driver.h files to remove
> style issues found by the checkpatch.pl script.
>
> +++ b/drivers/staging/rar/rar_driver.c
> @@ -68,7 +68,8 @@ static void __exit rar_exit_handler(void);
> /*
> function that is activated on the successfull probe of the RAR device
> */
> -static int __devinit rar_probe(struct pci_dev *pdev, const struct pci_device_id *ent);
> +static int __devinit rar_probe(struct pci_dev *pdev,
> + const struct pci_device_id *ent);
It's agreed such changes make it worse. The 80-column "ERROR" should be
ignored, and it will be removed from checkpatch.
> - printk(KERN_WARNING "rar- result from send ctl register is %x\n"
> - ,result);
> + printk(KERN_WARNING "rar- result from send ctl register is %x\n",
> + result);
Also, here (and then) - I'd just make it a single line if you're changing
it. I'd be far from "unwrapping" all code across the kernel, though
(without otherwise changing the lines in question).
> + if (memrar_get_rar_addr(pdev, (*i).low, &(rar_addr[n].low))
> + || memrar_get_rar_addr(pdev, (*i).high,
> + &(rar_addr[n].high))) {
> + result = -1;
> + break;
> + }
Isn't the following a bit more readable?
+ if (memrar_get_rar_addr(pdev, i->low, &rar_addr[n].low) ||
+ memrar_get_rar_addr(pdev, i->high, &rar_addr[n].high)) {
+ result = -1;
+ break;
+ }
It doesn't make sense to split the printk, at least every single output
line printed shouldn't be broken into pieces (but perhaps one single
line for the whole printk() is best).
Also I like the post-increments (z++) more, but maybe it's just me.
> + size_t z;
> + for (z = 0; z != MRST_NUM_RAR; ++z) {
> + printk(KERN_WARNING "rar - "
> + "BRAR[%Zd] physical address low\n"
> + "\tlow: 0x%08x\n"
> + "\thigh: 0x%08x\n",
> + z,
> + rar_addr[z].low,
> + rar_addr[z].high);
> -#define DEBUG_PRINT_0(DEBUG_LEVEL , info) \
> -do \
> -{ \
> - if(DEBUG_LEVEL) \
> - { \
> - printk(KERN_WARNING info); \
> - } \
> -}while(0)
> +#define DEBUG_PRINT_0(DEBUG_LEVEL, info) \
> +do { \
> + if (DEBUG_LEVEL) \
> + printk(KERN_WARNING info); \
> +} while (0)
Also I think moving these backslashes to the right of the macro code is
preferred, isn't it?
Just my 0.01$CURRENCY as usual :-)
--
Krzysztof Halasa
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
