[PATCH v2] Task2: checkpatch.pl documentation mentorship: Remove semicolon warnings

Rahul Balaji rahulbalaji78 at gmail.com
Mon Aug 9 20:57:25 UTC 2021


On Tue, 3 Aug 2021, 6:13 pm Rahul Balaji, <rahulbalaji78 at gmail.com> wrote:

> Respected mentor,
>
> Here are explanations for the changes I made.
>
>
> drivers/mtd/ubi/block.c:408: WARNING:ONE_SEMICOLON: Statements
> terminations use 1 semicolon
>
> removed the extra semicolon that caused the warning.
> No functional change.
> true positive.
> file showed no errors on make.
>
> checkpatch.pl on this file after change :
>
> WARNING: quoted string split across lines
> #354: FILE: drivers/mtd/ubi/block.c:354:
> + pr_warn("UBI: block: volume size is not a multiple of 512, "
> + "last %llu bytes are ignored!\n",
>
> WARNING: braces {} are not necessary for single statement blocks
> #374: FILE: drivers/mtd/ubi/block.c:374:
> + if (ret) {
> + return ret;
> + }
>
> total: 0 errors, 2 warnings, 697 lines checked
>
> drivers/gpu/drm/nouveau/nouveau_gem.c:342: WARNING:ONE_SEMICOLON:
> Statements terminations use 1 semicolon
>
> removed the extra semicolon that caused the warning.
> no functional change.
> true positive.
> file showed no errors on make.
>
> checkpatch.pl on this file after change :
>
> WARNING: Missing or malformed SPDX-License-Identifier tag in line 1
> #1: FILE: drivers/gpu/drm/nouveau/nouveau_gem.c:1:
> +/*
>
> ERROR: do not use assignment in if condition
> #165: FILE: drivers/gpu/drm/nouveau/nouveau_gem.c:165:
> + if (!(work = kmalloc(sizeof(*work), GFP_KERNEL))) {
>
> ERROR: space prohibited after that '&' (ctx:WxW)
> #183: FILE: drivers/gpu/drm/nouveau/nouveau_gem.c:183:
> + struct nouveau_vmm *vmm = cli->svm.cli ? &cli->svm : & cli->vmm;
>                                                       ^
>
> WARNING: Block comments use a trailing */ on a separate line
> #241: FILE: drivers/gpu/drm/nouveau/nouveau_gem.c:241:
> + * to the caller, instead of a normal nouveau_bo ttm reference. */
>
> WARNING: suspect code indent for conditional statements (8, 8)
> #298: FILE: drivers/gpu/drm/nouveau/nouveau_gem.c:298:
> + else
> + if (cli->device.info.family >= NV_DEVICE_INFO_V0_TESLA)
>
> WARNING: quoted string split across lines
> #458: FILE: drivers/gpu/drm/nouveau/nouveau_gem.c:458:
> + NV_PRINTK(err, cli, "multiple instances of buffer %d on "
> +      "validation list\n", b->handle);
>
> WARNING: Missing a blank line after declarations
> #486: FILE: drivers/gpu/drm/nouveau/nouveau_gem.c:486:
> + struct nouveau_vma *vma = nouveau_vma_find(nvbo, vmm);
> + if (!vma) {
>
> WARNING: suspect code indent for conditional statements (16, 16)
> #502: FILE: drivers/gpu/drm/nouveau/nouveau_gem.c:502:
> + else
> + if (b->valid_domains & NOUVEAU_GEM_DOMAIN_VRAM)
>
> WARNING: suspect code indent for conditional statements (16, 16)
> #505: FILE: drivers/gpu/drm/nouveau/nouveau_gem.c:505:
> + else
> + if (b->valid_domains & NOUVEAU_GEM_DOMAIN_GART)
>
> WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
> #625: FILE: drivers/gpu/drm/nouveau/nouveau_gem.c:625:
> +u_memcpya(uint64_t user, unsigned nmemb, unsigned size)
>
> WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
> #625: FILE: drivers/gpu/drm/nouveau/nouveau_gem.c:625:
> +u_memcpya(uint64_t user, unsigned nmemb, unsigned size)
>
> WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
> #651: FILE: drivers/gpu/drm/nouveau/nouveau_gem.c:651:
> + unsigned i;
>
> WARNING: suspect code indent for conditional statements (16, 16)
> #695: FILE: drivers/gpu/drm/nouveau/nouveau_gem.c:695:
> + else
> + if (r->flags & NOUVEAU_GEM_RELOC_HIGH)
>
> WARNING: suspect code indent for conditional statements (8, 8)
> #841: FILE: drivers/gpu/drm/nouveau/nouveau_gem.c:841:
> + } else
> + if (drm->client.device.info.chipset >= 0x25) {
>
> WARNING: Avoid multiple line dereference - prefer
> 'nvbo->bo.resource->num_pages'
> #874: FILE: drivers/gpu/drm/nouveau/nouveau_gem.c:874:
> +  nvbo->bo.resource->
> +  num_pages,
>
> ERROR: do not use assignment in if condition
> #902: FILE: drivers/gpu/drm/nouveau/nouveau_gem.c:902:
> + if (!(ret = nouveau_fence_wait(fence, false, false))) {
>
> ERROR: do not use assignment in if condition
> #903: FILE: drivers/gpu/drm/nouveau/nouveau_gem.c:903:
> + if ((ret = dma_fence_get_status(&fence->base)) == 1)
>
> WARNING: suspect code indent for conditional statements (8, 8)
> #937: FILE: drivers/gpu/drm/nouveau/nouveau_gem.c:937:
> + } else
> + if (drm->client.device.info.chipset >= 0x25) {
>
> total: 4 errors, 14 warnings, 1016 lines checked
>
> fs/block_dev.c:1271: WARNING:ONE_SEMICOLON: Statements terminations
> use 1 semicolon
>
> removed the semicolon that caused the warning.
> no functional changes.
> true positive.
> file showed no errors on make.
>
> checkpatch.pl on this file after change:
>
> WARNING: Missing a blank line after declarations
> #68: FILE: fs/block_dev.c:68:
> + char name[BDEVNAME_SIZE];
> + pr_warn_ratelimited("VFS: Dirty inode writeback failed "
>
> WARNING: quoted string split across lines
> #69: FILE: fs/block_dev.c:69:
> + pr_warn_ratelimited("VFS: Dirty inode writeback failed "
> +    "for block device %s (err=%d).\n",
>
> WARNING: Missing a blank line after declarations
> #120: FILE: fs/block_dev.c:120:
> + int err = bd_prepare_to_claim(bdev, truncate_bdev_range);
> + if (err)
>
> WARNING: EXPORT_SYMBOL(foo); should immediately follow its
> function/variable
> #171: FILE: fs/block_dev.c:171:
> +EXPORT_SYMBOL(set_blocksize);
>
> WARNING: Block comments use a trailing */ on a separate line
> #178: FILE: fs/block_dev.c:178:
> + * and it's value is between 512 and PAGE_SIZE */
>
> WARNING: EXPORT_SYMBOL(foo); should immediately follow its
> function/variable
> #184: FILE: fs/block_dev.c:184:
> +EXPORT_SYMBOL(sb_set_blocksize);
>
> WARNING: Missing a blank line after declarations
> #189: FILE: fs/block_dev.c:189:
> + int minsize = bdev_logical_block_size(sb->s_bdev);
> + if (size < minsize)
>
> WARNING: EXPORT_SYMBOL(foo); should immediately follow its
> function/variable
> #194: FILE: fs/block_dev.c:194:
> +EXPORT_SYMBOL(sb_min_blocksize);
>
> WARNING: Missing a blank line after declarations
> #547: FILE: fs/block_dev.c:547:
> + struct super_block *sb = get_super(bdev);
> + if (sb) {
>
> WARNING: Missing a blank line after declarations
> #549: FILE: fs/block_dev.c:549:
> + int res = sync_filesystem(sb);
> + drop_super(sb);
>
> ERROR: "foo * bar" should be "foo *bar"
> #642: FILE: fs/block_dev.c:642:
> +static int blkdev_readpage(struct file * file, struct page * page)
>
> ERROR: "foo * bar" should be "foo *bar"
> #642: FILE: fs/block_dev.c:642:
> +static int blkdev_readpage(struct file * file, struct page * page)
>
> WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
> #653: FILE: fs/block_dev.c:653:
> + loff_t pos, unsigned len, unsigned flags,
>
> WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
> #653: FILE: fs/block_dev.c:653:
> + loff_t pos, unsigned len, unsigned flags,
>
> WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
> #661: FILE: fs/block_dev.c:661:
> + loff_t pos, unsigned len, unsigned copied,
>
> WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
> #661: FILE: fs/block_dev.c:661:
> + loff_t pos, unsigned len, unsigned copied,
>
> WARNING: Missing a blank line after declarations
> #665: FILE: fs/block_dev.c:665:
> + int ret;
> + ret = block_write_end(file, mapping, pos, len, copied, page, fsdata);
>
> ERROR: trailing whitespace
> #688: FILE: fs/block_dev.c:688:
> +^I$
>
> ERROR: trailing whitespace
> #694: FILE: fs/block_dev.c:694:
> +^I$
>
> ERROR: "foo * bar" should be "foo *bar"
> #795: FILE: fs/block_dev.c:795:
> +static struct kmem_cache * bdev_cachep __read_mostly;
>
> WARNING: Missing a blank line after declarations
> #830: FILE: fs/block_dev.c:830:
> + struct block_device *bdev = &BDEV_I(inode)->bdev;
> + truncate_inode_pages_final(&inode->i_data);
>
> WARNING: Missing a blank line after declarations
> #852: FILE: fs/block_dev.c:852:
> + struct pseudo_fs_context *ctx = init_pseudo(fc, BDEVFS_MAGIC);
> + if (!ctx)
>
> ERROR: trailing whitespace
> #967: FILE: fs/block_dev.c:967:
> + $
>
> WARNING: please, no spaces at the start of a line
> #967: FILE: fs/block_dev.c:967:
> + $
>
> WARNING: please, no space before tabs
> #988: FILE: fs/block_dev.c:988:
> +^I^Ireturn false; ^I /* held by someone else */$
>
> WARNING: please, no space before tabs
> #990: FILE: fs/block_dev.c:990:
> +^I^Ireturn true;  ^I /* is a whole device which isn't held */$
>
> WARNING: please, no space before tabs
> #993: FILE: fs/block_dev.c:993:
> +^I^Ireturn true; ^I /* is a partition of a device that is being
> partitioned */$
>
> ERROR: "foo * bar" should be "foo *bar"
> #1491: FILE: fs/block_dev.c:1491:
> +static int blkdev_open(struct inode * inode, struct file * filp)
>
> ERROR: "foo * bar" should be "foo *bar"
> #1491: FILE: fs/block_dev.c:1491:
> +static int blkdev_open(struct inode * inode, struct file * filp)
>
> ERROR: do not use assignment in if condition
> #1549: FILE: fs/block_dev.c:1549:
> + if ((bdev_free = !bdev->bd_holders))
>
> ERROR: "foo * bar" should be "foo *bar"
> #1583: FILE: fs/block_dev.c:1583:
> +static int blkdev_close(struct inode * inode, struct file * filp)
>
> ERROR: "foo * bar" should be "foo *bar"
> #1583: FILE: fs/block_dev.c:1583:
> +static int blkdev_close(struct inode * inode, struct file * filp)
>
> WARNING: Missing a blank line after declarations
> #1586: FILE: fs/block_dev.c:1586:
> + struct block_device *bdev = I_BDEV(bdev_file_inode(filp));
> + blkdev_put(bdev, filp->f_mode);
>
> WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
> #1590: FILE: fs/block_dev.c:1590:
> +static long block_ioctl(struct file *file, unsigned cmd, unsigned long
> arg)
>
> total: 11 errors, 23 warnings, 1878 lines checked
>
> include/trace/events/iocost.h:163: WARNING:ONE_SEMICOLON: Statements
> terminations use 1 semicolon
>
> removed the extra semicolon that caused the warning.
> no functional change.
> true positive.
> showed no errors on "make -j8 all".
>
> checkpatch.pl after changes :
>
> WARNING: space prohibited between function name and open parenthesis '('
> #21: FILE: include/trace/events/iocost.h:21:
> + TP_STRUCT__entry (
>
> WARNING: quoted string split across lines
> #52: FILE: include/trace/events/iocost.h:52:
> + TP_printk("[%s:%s] now=%llu:%llu vrate=%llu "
> +  "period=%llu->%llu vtime=%llu "
>
> WARNING: quoted string split across lines
> #53: FILE: include/trace/events/iocost.h:53:
> +  "period=%llu->%llu vtime=%llu "
> +  "weight=%u/%u hweight=%llu/%llu",
>
> WARNING: space prohibited between function name and open parenthesis '('
> #85: FILE: include/trace/events/iocost.h:85:
> + TP_STRUCT__entry (
>
> WARNING: space prohibited between function name and open parenthesis '('
> #149: FILE: include/trace/events/iocost.h:149:
> + TP_STRUCT__entry (
>
> WARNING: space prohibited between function name and open parenthesis '('
> #190: FILE: include/trace/events/iocost.h:190:
> + TP_STRUCT__entry (
>
> total: 0 errors, 6 warnings, 225 lines checked
>
> net/sched/cls_rsvp.h:231: WARNING:ONE_SEMICOLON: Statements
> terminations use 1 semicolon
>
> false positive.
> no changes made.
> a for loop will require 2 semicolons as per the rules of C.
> we can maybe have a check for this rule where it can be ignored when
> it is in a for loop ? or have a check for the number of consecutive
> semicolons be less than 2.
>
> net/sched/cls_u32.c:782: WARNING:ONE_SEMICOLON: Statements
> terminations use 1 semicolon
>
> false positive.
> no changes made.
> a for loop will require 2 semicolons as per the rules of C.
>
> drivers/net/ethernet/mediatek/mtk_star_emac.c:1089:
> WARNING:ONE_SEMICOLON: Statements terminations use 1 semicolon
>
> false positive.
> no changes made.
> a for loop will require 2 semicolons as per the rules of C.
>
> arch/ia64/include/asm/mca_asm.h:139: WARNING:ONE_SEMICOLON: Statements
> terminations use 1 semicolon
>
> arch/ia64/include/asm/mca_asm.h:213: WARNING:ONE_SEMICOLON: Statements
> terminations use 1 semicolon
>
> arch/ia64/include/asm/native/inst.h:79: WARNING:ONE_SEMICOLON:
> Statements terminations use 1 semicolon
>
> arch/ia64/kernel/minstate.h:14: WARNING:ONE_SEMICOLON: Statements
> terminations use 1 semicolon
>
> arch/ia64/kernel/minstate.h:151: WARNING:ONE_SEMICOLON: Statements
> terminations use 1 semicolon
>
> arch/ia64/kernel/minstate.h:213: WARNING:ONE_SEMICOLON: Statements
> terminations use 1 semicolon
>
> false positive.
> i am not entirely sure of this rule,
> but what i noticed is that the place where ";;"
> is used is all under #define. I am not entirely sure of it's
> function in c under .h files. Can you help me out on that one ?
> but other than that, i am positive it shouldn't be removed,
> or else the function might get altered or it might result in an error.
>

Respected mentors,

please do find the  explanations to my changes quoted above.
I had already explained them in my previous mail, maybe this was not
present in the previous mail due to some issue.

Regards,
Rahul.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linuxfoundation.org/pipermail/linux-kernel-mentees/attachments/20210810/71e7571c/attachment-0001.html>


More information about the Linux-kernel-mentees mailing list