From 879be4f378cb412af3a3fe107d35835c99099add Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Fri, 3 Jun 2016 14:55:49 -0700 Subject: checkpatch: reduce git commit description style false positives Some lines in a commit log appear to be commit SHA1 ids like: ERROR: Please use git commit description style 'commit <12+ chars of sha1> ("")' - ie: 'commit 0123456789ab ("commit description")' Link: http://lkml.kernel.org/r/40e03fd7aaf1f55c75d787128d6d17c5a71226c2.1464358556.git.vdavydov@virtuozzo.com Reduce the false positives. Link: http://lkml.kernel.org/r/eda977eaa8328fef42bb3c87935d97e10ea8ff67.1464384023.git.joe@perches.com Signed-off-by: Joe Perches <joe@perches.com> Reported-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> --- scripts/checkpatch.pl | 1 + 1 file changed, 1 insertion(+) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 6750595bd7b8..4904ced676d4 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2454,6 +2454,7 @@ sub process { # Check for git id commit length and improperly formed commit descriptions if ($in_commit_log && !$commit_log_possible_stack_dump && + $line !~ /^\s*(?:Link|Patchwork|http|BugLink):/i && ($line =~ /\bcommit\s+[0-9a-f]{5,}\b/i || ($line =~ /\b[0-9a-f]{12,40}\b/i && $line !~ /[\<\[][0-9a-f]{12,40}[\>\]]/i && -- cgit v1.2.3 From 7a9eb20666317794d0279843fbd091af93907780 Mon Sep 17 00:00:00 2001 From: Dan Williams <dan.j.williams@intel.com> Date: Fri, 3 Jun 2016 18:06:47 -0700 Subject: pmem: kill __pmem address space The __pmem address space was meant to annotate codepaths that touch persistent memory and need to coordinate a call to wmb_pmem(). Now that wmb_pmem() is gone, there is little need to keep this annotation. Cc: Christoph Hellwig <hch@lst.de> Cc: Ross Zwisler <ross.zwisler@linux.intel.com> Signed-off-by: Dan Williams <dan.j.williams@intel.com> --- Documentation/filesystems/Locking | 2 +- arch/powerpc/sysdev/axonram.c | 4 +-- arch/x86/include/asm/pmem.h | 41 ++++++++++------------- drivers/acpi/nfit.h | 2 +- drivers/block/brd.c | 4 +-- drivers/nvdimm/pmem.c | 6 ++-- drivers/nvdimm/pmem.h | 4 +-- drivers/s390/block/dcssblk.c | 6 ++-- fs/dax.c | 6 ++-- include/linux/blkdev.h | 6 ++-- include/linux/compiler.h | 2 -- include/linux/nd.h | 2 +- include/linux/pmem.h | 70 ++++++++++----------------------------- scripts/checkpatch.pl | 1 - tools/testing/nvdimm/pmem-dax.c | 2 +- 15 files changed, 56 insertions(+), 102 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/Documentation/filesystems/Locking b/Documentation/filesystems/Locking index 75eea7ce3d7c..d9c37ec4c760 100644 --- a/Documentation/filesystems/Locking +++ b/Documentation/filesystems/Locking @@ -395,7 +395,7 @@ prototypes: int (*release) (struct gendisk *, fmode_t); int (*ioctl) (struct block_device *, fmode_t, unsigned, unsigned long); int (*compat_ioctl) (struct block_device *, fmode_t, unsigned, unsigned long); - int (*direct_access) (struct block_device *, sector_t, void __pmem **, + int (*direct_access) (struct block_device *, sector_t, void **, unsigned long *); int (*media_changed) (struct gendisk *); void (*unlock_native_capacity) (struct gendisk *); diff --git a/arch/powerpc/sysdev/axonram.c b/arch/powerpc/sysdev/axonram.c index ff75d70f7285..a87489d007dc 100644 --- a/arch/powerpc/sysdev/axonram.c +++ b/arch/powerpc/sysdev/axonram.c @@ -143,12 +143,12 @@ axon_ram_make_request(struct request_queue *queue, struct bio *bio) */ static long axon_ram_direct_access(struct block_device *device, sector_t sector, - void __pmem **kaddr, pfn_t *pfn, long size) + void **kaddr, pfn_t *pfn, long size) { struct axon_ram_bank *bank = device->bd_disk->private_data; loff_t offset = (loff_t)sector << AXON_RAM_SECTOR_SHIFT; - *kaddr = (void __pmem __force *) bank->io_addr + offset; + *kaddr = (void *) bank->io_addr + offset; *pfn = phys_to_pfn_t(bank->ph_addr + offset, PFN_DEV); return bank->size - offset; } diff --git a/arch/x86/include/asm/pmem.h b/arch/x86/include/asm/pmem.h index a8cf2a6b14d9..643eba42d620 100644 --- a/arch/x86/include/asm/pmem.h +++ b/arch/x86/include/asm/pmem.h @@ -28,10 +28,9 @@ * Copy data to persistent memory media via non-temporal stores so that * a subsequent pmem driver flush operation will drain posted write queues. */ -static inline void arch_memcpy_to_pmem(void __pmem *dst, const void *src, - size_t n) +static inline void arch_memcpy_to_pmem(void *dst, const void *src, size_t n) { - int unwritten; + int rem; /* * We are copying between two kernel buffers, if @@ -39,19 +38,17 @@ static inline void arch_memcpy_to_pmem(void __pmem *dst, const void *src, * fault) we would have already reported a general protection fault * before the WARN+BUG. */ - unwritten = __copy_from_user_inatomic_nocache((void __force *) dst, - (void __user *) src, n); - if (WARN(unwritten, "%s: fault copying %p <- %p unwritten: %d\n", - __func__, dst, src, unwritten)) + rem = __copy_from_user_inatomic_nocache(dst, (void __user *) src, n); + if (WARN(rem, "%s: fault copying %p <- %p unwritten: %d\n", + __func__, dst, src, rem)) BUG(); } -static inline int arch_memcpy_from_pmem(void *dst, const void __pmem *src, - size_t n) +static inline int arch_memcpy_from_pmem(void *dst, const void *src, size_t n) { if (static_cpu_has(X86_FEATURE_MCE_RECOVERY)) - return memcpy_mcsafe(dst, (void __force *) src, n); - memcpy(dst, (void __force *) src, n); + return memcpy_mcsafe(dst, src, n); + memcpy(dst, src, n); return 0; } @@ -63,15 +60,14 @@ static inline int arch_memcpy_from_pmem(void *dst, const void __pmem *src, * Write back a cache range using the CLWB (cache line write back) * instruction. */ -static inline void arch_wb_cache_pmem(void __pmem *addr, size_t size) +static inline void arch_wb_cache_pmem(void *addr, size_t size) { u16 x86_clflush_size = boot_cpu_data.x86_clflush_size; unsigned long clflush_mask = x86_clflush_size - 1; - void *vaddr = (void __force *)addr; - void *vend = vaddr + size; + void *vend = addr + size; void *p; - for (p = (void *)((unsigned long)vaddr & ~clflush_mask); + for (p = (void *)((unsigned long)addr & ~clflush_mask); p < vend; p += x86_clflush_size) clwb(p); } @@ -93,14 +89,13 @@ static inline bool __iter_needs_pmem_wb(struct iov_iter *i) * * Copy data from the iterator 'i' to the PMEM buffer starting at 'addr'. */ -static inline size_t arch_copy_from_iter_pmem(void __pmem *addr, size_t bytes, +static inline size_t arch_copy_from_iter_pmem(void *addr, size_t bytes, struct iov_iter *i) { - void *vaddr = (void __force *)addr; size_t len; /* TODO: skip the write-back by always using non-temporal stores */ - len = copy_from_iter_nocache(vaddr, bytes, i); + len = copy_from_iter_nocache(addr, bytes, i); if (__iter_needs_pmem_wb(i)) arch_wb_cache_pmem(addr, bytes); @@ -115,17 +110,15 @@ static inline size_t arch_copy_from_iter_pmem(void __pmem *addr, size_t bytes, * * Write zeros into the memory range starting at 'addr' for 'size' bytes. */ -static inline void arch_clear_pmem(void __pmem *addr, size_t size) +static inline void arch_clear_pmem(void *addr, size_t size) { - void *vaddr = (void __force *)addr; - - memset(vaddr, 0, size); + memset(addr, 0, size); arch_wb_cache_pmem(addr, size); } -static inline void arch_invalidate_pmem(void __pmem *addr, size_t size) +static inline void arch_invalidate_pmem(void *addr, size_t size) { - clflush_cache_range((void __force *) addr, size); + clflush_cache_range(addr, size); } #endif /* CONFIG_ARCH_HAS_PMEM_API */ #endif /* __ASM_X86_PMEM_H__ */ diff --git a/drivers/acpi/nfit.h b/drivers/acpi/nfit.h index 9fda77cf81da..80fb2c0ac8bf 100644 --- a/drivers/acpi/nfit.h +++ b/drivers/acpi/nfit.h @@ -164,7 +164,7 @@ enum nd_blk_mmio_selector { struct nd_blk_addr { union { void __iomem *base; - void __pmem *aperture; + void *aperture; }; }; diff --git a/drivers/block/brd.c b/drivers/block/brd.c index c04bd9bc39fd..5f1fe4e6208d 100644 --- a/drivers/block/brd.c +++ b/drivers/block/brd.c @@ -381,7 +381,7 @@ static int brd_rw_page(struct block_device *bdev, sector_t sector, #ifdef CONFIG_BLK_DEV_RAM_DAX static long brd_direct_access(struct block_device *bdev, sector_t sector, - void __pmem **kaddr, pfn_t *pfn, long size) + void **kaddr, pfn_t *pfn, long size) { struct brd_device *brd = bdev->bd_disk->private_data; struct page *page; @@ -391,7 +391,7 @@ static long brd_direct_access(struct block_device *bdev, sector_t sector, page = brd_insert_page(brd, sector); if (!page) return -ENOSPC; - *kaddr = (void __pmem *)page_address(page); + *kaddr = page_address(page); *pfn = page_to_pfn_t(page); return PAGE_SIZE; diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c index 8bfc6acc2e43..7251b4b6da84 100644 --- a/drivers/nvdimm/pmem.c +++ b/drivers/nvdimm/pmem.c @@ -74,7 +74,7 @@ static int pmem_do_bvec(struct pmem_device *pmem, struct page *page, bool bad_pmem = false; void *mem = kmap_atomic(page); phys_addr_t pmem_off = sector * 512 + pmem->data_offset; - void __pmem *pmem_addr = pmem->virt_addr + pmem_off; + void *pmem_addr = pmem->virt_addr + pmem_off; if (unlikely(is_bad_pmem(&pmem->bb, sector, len))) bad_pmem = true; @@ -173,7 +173,7 @@ static int pmem_rw_page(struct block_device *bdev, sector_t sector, /* see "strong" declaration in tools/testing/nvdimm/pmem-dax.c */ __weak long pmem_direct_access(struct block_device *bdev, sector_t sector, - void __pmem **kaddr, pfn_t *pfn, long size) + void **kaddr, pfn_t *pfn, long size) { struct pmem_device *pmem = bdev->bd_queue->queuedata; resource_size_t offset = sector * 512 + pmem->data_offset; @@ -284,7 +284,7 @@ static int pmem_attach_disk(struct device *dev, if (IS_ERR(addr)) return PTR_ERR(addr); - pmem->virt_addr = (void __pmem *) addr; + pmem->virt_addr = addr; blk_queue_write_cache(q, true, true); blk_queue_make_request(q, pmem_make_request); diff --git a/drivers/nvdimm/pmem.h b/drivers/nvdimm/pmem.h index c48d4e3aa346..b4ee4f71b4a1 100644 --- a/drivers/nvdimm/pmem.h +++ b/drivers/nvdimm/pmem.h @@ -6,7 +6,7 @@ #include <linux/fs.h> long pmem_direct_access(struct block_device *bdev, sector_t sector, - void __pmem **kaddr, pfn_t *pfn, long size); + void **kaddr, pfn_t *pfn, long size); /* this definition is in it's own header for tools/testing/nvdimm to consume */ struct pmem_device { /* One contiguous memory region per device */ @@ -14,7 +14,7 @@ struct pmem_device { /* when non-zero this device is hosting a 'pfn' instance */ phys_addr_t data_offset; u64 pfn_flags; - void __pmem *virt_addr; + void *virt_addr; /* immutable base size of the namespace */ size_t size; /* trim size when namespace capacity has been section aligned */ diff --git a/drivers/s390/block/dcssblk.c b/drivers/s390/block/dcssblk.c index bed53c46dd90..023c5c975dc0 100644 --- a/drivers/s390/block/dcssblk.c +++ b/drivers/s390/block/dcssblk.c @@ -31,7 +31,7 @@ static void dcssblk_release(struct gendisk *disk, fmode_t mode); static blk_qc_t dcssblk_make_request(struct request_queue *q, struct bio *bio); static long dcssblk_direct_access(struct block_device *bdev, sector_t secnum, - void __pmem **kaddr, pfn_t *pfn, long size); + void **kaddr, pfn_t *pfn, long size); static char dcssblk_segments[DCSSBLK_PARM_LEN] = "\0"; @@ -884,7 +884,7 @@ fail: static long dcssblk_direct_access (struct block_device *bdev, sector_t secnum, - void __pmem **kaddr, pfn_t *pfn, long size) + void **kaddr, pfn_t *pfn, long size) { struct dcssblk_dev_info *dev_info; unsigned long offset, dev_sz; @@ -894,7 +894,7 @@ dcssblk_direct_access (struct block_device *bdev, sector_t secnum, return -ENODEV; dev_sz = dev_info->end - dev_info->start; offset = secnum * 512; - *kaddr = (void __pmem *) (dev_info->start + offset); + *kaddr = (void *) dev_info->start + offset; *pfn = __pfn_to_pfn_t(PFN_DOWN(dev_info->start + offset), PFN_DEV); return dev_sz - offset; diff --git a/fs/dax.c b/fs/dax.c index 434f421da660..c8312f6441bc 100644 --- a/fs/dax.c +++ b/fs/dax.c @@ -75,13 +75,13 @@ static long dax_map_atomic(struct block_device *bdev, struct blk_dax_ctl *dax) struct request_queue *q = bdev->bd_queue; long rc = -EIO; - dax->addr = (void __pmem *) ERR_PTR(-EIO); + dax->addr = ERR_PTR(-EIO); if (blk_queue_enter(q, true) != 0) return rc; rc = bdev_direct_access(bdev, dax); if (rc < 0) { - dax->addr = (void __pmem *) ERR_PTR(rc); + dax->addr = ERR_PTR(rc); blk_queue_exit(q); return rc; } @@ -152,7 +152,7 @@ static ssize_t dax_io(struct inode *inode, struct iov_iter *iter, int rw = iov_iter_rw(iter), rc; long map_len = 0; struct blk_dax_ctl dax = { - .addr = (void __pmem *) ERR_PTR(-EIO), + .addr = ERR_PTR(-EIO), }; unsigned blkbits = inode->i_blkbits; sector_t file_blks = (i_size_read(inode) + (1 << blkbits) - 1) diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h index 3d9cf326574f..fde908b2836b 100644 --- a/include/linux/blkdev.h +++ b/include/linux/blkdev.h @@ -1659,7 +1659,7 @@ static inline bool integrity_req_gap_front_merge(struct request *req, */ struct blk_dax_ctl { sector_t sector; - void __pmem *addr; + void *addr; long size; pfn_t pfn; }; @@ -1670,8 +1670,8 @@ struct block_device_operations { int (*rw_page)(struct block_device *, sector_t, struct page *, int rw); int (*ioctl) (struct block_device *, fmode_t, unsigned, unsigned long); int (*compat_ioctl) (struct block_device *, fmode_t, unsigned, unsigned long); - long (*direct_access)(struct block_device *, sector_t, void __pmem **, - pfn_t *, long); + long (*direct_access)(struct block_device *, sector_t, void **, pfn_t *, + long); unsigned int (*check_events) (struct gendisk *disk, unsigned int clearing); /* ->media_changed() is DEPRECATED, use ->check_events() instead */ diff --git a/include/linux/compiler.h b/include/linux/compiler.h index 793c0829e3a3..b966974938ed 100644 --- a/include/linux/compiler.h +++ b/include/linux/compiler.h @@ -17,7 +17,6 @@ # define __release(x) __context__(x,-1) # define __cond_lock(x,c) ((c) ? ({ __acquire(x); 1; }) : 0) # define __percpu __attribute__((noderef, address_space(3))) -# define __pmem __attribute__((noderef, address_space(5))) #ifdef CONFIG_SPARSE_RCU_POINTER # define __rcu __attribute__((noderef, address_space(4))) #else /* CONFIG_SPARSE_RCU_POINTER */ @@ -45,7 +44,6 @@ extern void __chk_io_ptr(const volatile void __iomem *); # define __cond_lock(x,c) (c) # define __percpu # define __rcu -# define __pmem # define __private # define ACCESS_PRIVATE(p, member) ((p)->member) #endif /* __CHECKER__ */ diff --git a/include/linux/nd.h b/include/linux/nd.h index 1ecd64643512..f1ea426d6a5e 100644 --- a/include/linux/nd.h +++ b/include/linux/nd.h @@ -68,7 +68,7 @@ struct nd_namespace_io { struct nd_namespace_common common; struct resource res; resource_size_t size; - void __pmem *addr; + void *addr; struct badblocks bb; }; diff --git a/include/linux/pmem.h b/include/linux/pmem.h index 9e3ea94b8157..e856c2cb0fe8 100644 --- a/include/linux/pmem.h +++ b/include/linux/pmem.h @@ -26,37 +26,35 @@ * calling these symbols with arch_has_pmem_api() and redirect to the * implementation in asm/pmem.h. */ -static inline void arch_memcpy_to_pmem(void __pmem *dst, const void *src, - size_t n) +static inline void arch_memcpy_to_pmem(void *dst, const void *src, size_t n) { BUG(); } -static inline int arch_memcpy_from_pmem(void *dst, const void __pmem *src, - size_t n) +static inline int arch_memcpy_from_pmem(void *dst, const void *src, size_t n) { BUG(); return -EFAULT; } -static inline size_t arch_copy_from_iter_pmem(void __pmem *addr, size_t bytes, +static inline size_t arch_copy_from_iter_pmem(void *addr, size_t bytes, struct iov_iter *i) { BUG(); return 0; } -static inline void arch_clear_pmem(void __pmem *addr, size_t size) +static inline void arch_clear_pmem(void *addr, size_t size) { BUG(); } -static inline void arch_wb_cache_pmem(void __pmem *addr, size_t size) +static inline void arch_wb_cache_pmem(void *addr, size_t size) { BUG(); } -static inline void arch_invalidate_pmem(void __pmem *addr, size_t size) +static inline void arch_invalidate_pmem(void *addr, size_t size) { BUG(); } @@ -67,13 +65,6 @@ static inline bool arch_has_pmem_api(void) return IS_ENABLED(CONFIG_ARCH_HAS_PMEM_API); } -static inline int default_memcpy_from_pmem(void *dst, void __pmem const *src, - size_t size) -{ - memcpy(dst, (void __force *) src, size); - return 0; -} - /* * memcpy_from_pmem - read from persistent memory with error handling * @dst: destination buffer @@ -82,40 +73,13 @@ static inline int default_memcpy_from_pmem(void *dst, void __pmem const *src, * * Returns 0 on success negative error code on failure. */ -static inline int memcpy_from_pmem(void *dst, void __pmem const *src, - size_t size) +static inline int memcpy_from_pmem(void *dst, void const *src, size_t size) { if (arch_has_pmem_api()) return arch_memcpy_from_pmem(dst, src, size); else - return default_memcpy_from_pmem(dst, src, size); -} - -/* - * These defaults seek to offer decent performance and minimize the - * window between i/o completion and writes being durable on media. - * However, it is undefined / architecture specific whether - * ARCH_MEMREMAP_PMEM + default_memcpy_to_pmem is sufficient for - * making data durable relative to i/o completion. - */ -static inline void default_memcpy_to_pmem(void __pmem *dst, const void *src, - size_t size) -{ - memcpy((void __force *) dst, src, size); -} - -static inline size_t default_copy_from_iter_pmem(void __pmem *addr, - size_t bytes, struct iov_iter *i) -{ - return copy_from_iter_nocache((void __force *)addr, bytes, i); -} - -static inline void default_clear_pmem(void __pmem *addr, size_t size) -{ - if (size == PAGE_SIZE && ((unsigned long)addr & ~PAGE_MASK) == 0) - clear_page((void __force *)addr); - else - memset((void __force *)addr, 0, size); + memcpy(dst, src, size); + return 0; } /** @@ -130,12 +94,12 @@ static inline void default_clear_pmem(void __pmem *addr, size_t size) * data may still reside in cpu or platform buffers, so this operation * must be followed by a blkdev_issue_flush() on the pmem block device. */ -static inline void memcpy_to_pmem(void __pmem *dst, const void *src, size_t n) +static inline void memcpy_to_pmem(void *dst, const void *src, size_t n) { if (arch_has_pmem_api()) arch_memcpy_to_pmem(dst, src, n); else - default_memcpy_to_pmem(dst, src, n); + memcpy(dst, src, n); } /** @@ -147,12 +111,12 @@ static inline void memcpy_to_pmem(void __pmem *dst, const void *src, size_t n) * Copy data from the iterator 'i' to the PMEM buffer starting at 'addr'. * See blkdev_issue_flush() note for memcpy_to_pmem(). */ -static inline size_t copy_from_iter_pmem(void __pmem *addr, size_t bytes, +static inline size_t copy_from_iter_pmem(void *addr, size_t bytes, struct iov_iter *i) { if (arch_has_pmem_api()) return arch_copy_from_iter_pmem(addr, bytes, i); - return default_copy_from_iter_pmem(addr, bytes, i); + return copy_from_iter_nocache(addr, bytes, i); } /** @@ -163,12 +127,12 @@ static inline size_t copy_from_iter_pmem(void __pmem *addr, size_t bytes, * Write zeros into the memory range starting at 'addr' for 'size' bytes. * See blkdev_issue_flush() note for memcpy_to_pmem(). */ -static inline void clear_pmem(void __pmem *addr, size_t size) +static inline void clear_pmem(void *addr, size_t size) { if (arch_has_pmem_api()) arch_clear_pmem(addr, size); else - default_clear_pmem(addr, size); + memset(addr, 0, size); } /** @@ -179,7 +143,7 @@ static inline void clear_pmem(void __pmem *addr, size_t size) * For platforms that support clearing poison this flushes any poisoned * ranges out of the cache */ -static inline void invalidate_pmem(void __pmem *addr, size_t size) +static inline void invalidate_pmem(void *addr, size_t size) { if (arch_has_pmem_api()) arch_invalidate_pmem(addr, size); @@ -193,7 +157,7 @@ static inline void invalidate_pmem(void __pmem *addr, size_t size) * Write back the processor cache range starting at 'addr' for 'size' bytes. * See blkdev_issue_flush() note for memcpy_to_pmem(). */ -static inline void wb_cache_pmem(void __pmem *addr, size_t size) +static inline void wb_cache_pmem(void *addr, size_t size) { if (arch_has_pmem_api()) arch_wb_cache_pmem(addr, size); diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 4904ced676d4..24a08363995a 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -313,7 +313,6 @@ our $Sparse = qr{ __kernel| __force| __iomem| - __pmem| __must_check| __init_refok| __kprobes| diff --git a/tools/testing/nvdimm/pmem-dax.c b/tools/testing/nvdimm/pmem-dax.c index 1e0218ce6a8b..c9b8c48f85fc 100644 --- a/tools/testing/nvdimm/pmem-dax.c +++ b/tools/testing/nvdimm/pmem-dax.c @@ -16,7 +16,7 @@ #include <nd.h> long pmem_direct_access(struct block_device *bdev, sector_t sector, - void __pmem **kaddr, pfn_t *pfn, long size) + void **kaddr, pfn_t *pfn, long size) { struct pmem_device *pmem = bdev->bd_queue->queuedata; resource_size_t offset = sector * 512 + pmem->data_offset; -- cgit v1.2.3 From d560a5f8a46e98c2f83c5fe699e1d6f6393a14cf Mon Sep 17 00:00:00 2001 From: Joe Perches <joe@perches.com> Date: Tue, 2 Aug 2016 14:04:31 -0700 Subject: checkpatch: skip long lines that use an EFI_GUID macro These are also possible single line uses that exceed the generic maximum line length (typically 80 columns) Link: http://lkml.kernel.org/r/32a6a85fbd6161f1bb55ce176a464e44591afc5b.1468368420.git.joe@perches.com Signed-off-by: Joe Perches <joe@perches.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> --- scripts/checkpatch.pl | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 24a08363995a..a4476b61e93f 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2763,6 +2763,10 @@ sub process { $line =~ /^\+\s*#\s*define\s+\w+\s+$String$/) { $msg_type = ""; + # EFI_GUID is another special case + } elsif ($line =~ /^\+.*\bEFI_GUID\s*\(/) { + $msg_type = ""; + # Otherwise set the alternate message types # a comment starts before $max_line_length -- cgit v1.2.3 From dadf680de3c2eb4cba9840619991eda0cfe98778 Mon Sep 17 00:00:00 2001 From: Joe Perches <joe@perches.com> Date: Tue, 2 Aug 2016 14:04:33 -0700 Subject: checkpatch: allow c99 style // comments Sanitise the lines that contain c99 comments so that the error doesn't get emitted. Link: http://lkml.kernel.org/r/d4d22c34ad7bcc1bceb52f0742f76b7a6d585235.1468368420.git.joe@perches.com Signed-off-by: Joe Perches <joe@perches.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> --- scripts/checkpatch.pl | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index a4476b61e93f..79273003d5e7 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -55,6 +55,7 @@ my $spelling_file = "$D/spelling.txt"; my $codespell = 0; my $codespellfile = "/usr/share/codespell/dictionary.txt"; my $color = 1; +my $allow_c99_comments = 1; sub help { my ($exitcode) = @_; @@ -1144,6 +1145,11 @@ sub sanitise_line { $res =~ s@(\#\s*(?:error|warning)\s+).*@$1$clean@; } + if ($allow_c99_comments && $res =~ m@(//.*$)@) { + my $match = $1; + $res =~ s/\Q$match\E/"$;" x length($match)/e; + } + return $res; } -- cgit v1.2.3 From aab38f516aa99e8132f906a526bf44fa59e9daa3 Mon Sep 17 00:00:00 2001 From: Joe Perches <joe@perches.com> Date: Tue, 2 Aug 2016 14:04:36 -0700 Subject: checkpatch: yet another commit id improvement Using \b isn't good enough to isolate what appears to be a commit id in a commit message. Make sure there is a space or a quote like character after a continuous run of hexadecimal characters that could be a commit id. Link: http://lkml.kernel.org/r/fdd22b47463a21c21132edbb8aa35e372950a1e6.1468869915.git.joe@perches.com Signed-off-by: Joe Perches <joe@perches.com> Cc: "Zhuo, Qiuxu" <qiuxu.zhuo@intel.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> --- scripts/checkpatch.pl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 79273003d5e7..7a28775274a5 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2459,9 +2459,9 @@ sub process { # Check for git id commit length and improperly formed commit descriptions if ($in_commit_log && !$commit_log_possible_stack_dump && - $line !~ /^\s*(?:Link|Patchwork|http|BugLink):/i && + $line !~ /^\s*(?:Link|Patchwork|http|https|BugLink):/i && ($line =~ /\bcommit\s+[0-9a-f]{5,}\b/i || - ($line =~ /\b[0-9a-f]{12,40}\b/i && + ($line =~ /(?:\s|^)[0-9a-f]{12,40}(?:[\s"'\(\[]|$)/i && $line !~ /[\<\[][0-9a-f]{12,40}[\>\]]/i && $line !~ /\bfixes:\s*[0-9a-f]{12,40}/i))) { my $init_char = "c"; -- cgit v1.2.3 From cec3aaa56638c7aad763630b9cbe591f2e791a3b Mon Sep 17 00:00:00 2001 From: Tomas Winkler <tomas.winkler@intel.com> Date: Tue, 2 Aug 2016 14:04:39 -0700 Subject: checkpatch: don't complain about BIT macro in uapi BIT macro cannot be exported to UAPI, don't complain about it. Link: http://lkml.kernel.org/r/1468707033-16173-1-git-send-email-tomas.winkler@intel.com Signed-off-by: Tomas Winkler <tomas.winkler@intel.com> Acked-by: Joe Perches <joe@perches.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> --- scripts/checkpatch.pl | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 7a28775274a5..77915e095022 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -5732,8 +5732,9 @@ sub process { } } -# check for #defines like: 1 << <digit> that could be BIT(digit) - if ($line =~ /#\s*define\s+\w+\s+\(?\s*1\s*([ulUL]*)\s*\<\<\s*(?:\d+|$Ident)\s*\)?/) { +# check for #defines like: 1 << <digit> that could be BIT(digit), it is not exported to uapi + if ($realfile !~ m@^include/uapi/@ && + $line =~ /#\s*define\s+\w+\s+\(?\s*1\s*([ulUL]*)\s*\<\<\s*(?:\d+|$Ident)\s*\)?/) { my $ull = ""; $ull = "_ULL" if (defined($1) && $1 =~ /ll/i); if (CHK("BIT_MACRO", -- cgit v1.2.3 From c844711575086231890084390a275d06f11a623a Mon Sep 17 00:00:00 2001 From: Joe Perches <joe@perches.com> Date: Tue, 2 Aug 2016 14:04:42 -0700 Subject: checkpatch: improve 'bare use of' signed/unsigned types warning Fix false positive warning of identifiers ending in signed with an = assignment of WARNING: Prefer 'signed int' to bare use of 'signed'. Link: http://lkml.kernel.org/r/6a0e24c3e9102337528ecfcbbe91a0eb5b4820ed.1469529497.git.joe@perches.com Signed-off-by: Joe Perches <joe@perches.com> Reported-by: Alan Douglas <alanjhd@gmail.com> Acked-by: Andy Whitcroft <apw@canonical.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> --- scripts/checkpatch.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 77915e095022..1d5b09dd577a 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -3347,7 +3347,7 @@ sub process { next if ($line =~ /^[^\+]/); # check for declarations of signed or unsigned without int - while ($line =~ m{($Declare)\s*(?!char\b|short\b|int\b|long\b)\s*($Ident)?\s*[=,;\[\)\(]}g) { + while ($line =~ m{\b($Declare)\s*(?!char\b|short\b|int\b|long\b)\s*($Ident)?\s*[=,;\[\)\(]}g) { my $type = $1; my $var = $2; $var = "" if (!defined $var); -- cgit v1.2.3 From ed43c4e58a6d3061e3329c41d7b880f11541245a Mon Sep 17 00:00:00 2001 From: Allen Hubbe <allenbh@gmail.com> Date: Tue, 2 Aug 2016 14:04:45 -0700 Subject: checkpatch: check signoff when reading stdin Signoff was not checked if the filename is '-', indicating reading the patch from stdin. Commands such as the below would not warn about a missing signoff, because the patch filename is '-'. This change allows checkpatch to warn about a missing signoff, even if the input filename is '-', but only if the patch has a commit message. git show --pretty=email | scripts/checkpatch.pl - A more common use of checkpatch with stdin is for piping git diff through checkpatch. The diff output would not contain a commit message, and therefore it would not contain a signoff line. For this common use case, a warning should not be printed about the missing signoff. With this change we will only warn about a missing signoff if the input contains a commit message. git diff | scripts/checkpatch.pl - Before this patch, a workaround for the first command was to refer to stdin by a name other than '-'. The workaround is not an elegant solution, because elsewhere checkpatch uses the fact that filename equals '-', such as in setting '$vname' to 'Your patch' for stdin. The command below would report "/dev/stdin has style problems" instead of "Your patch has style problems." git show --pretty=email | scripts/checkpatch.pl /dev/stdin Link: http://lkml.kernel.org/r/48be31e414bddc65bccfa6b1322359be9ba032eb.1469670589.git.allenbh@gmail.com Signed-off-by: Allen Hubbe <allenbh@gmail.com> Acked-by: Joe Perches <joe@perches.com> Cc: Andy Whitcroft <apw@canonical.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> --- scripts/checkpatch.pl | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 1d5b09dd577a..6f2ce0cafe6f 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2069,6 +2069,7 @@ sub process { my $is_patch = 0; my $in_header_lines = $file ? 0 : 1; my $in_commit_log = 0; #Scanning lines before patch + my $has_commit_log = 0; #Encountered lines before patch my $commit_log_possible_stack_dump = 0; my $commit_log_long_line = 0; my $commit_log_has_diff = 0; @@ -2566,6 +2567,7 @@ sub process { $rawline =~ /^(commit\b|from\b|[\w-]+:).*$/i)) { $in_header_lines = 0; $in_commit_log = 1; + $has_commit_log = 1; } # Check if there is UTF-8 in a commit log when a mail header has explicitly @@ -6055,7 +6057,7 @@ sub process { ERROR("NOT_UNIFIED_DIFF", "Does not appear to be a unified-diff format patch\n"); } - if ($is_patch && $filename ne '-' && $chk_signoff && $signoff == 0) { + if ($is_patch && $has_commit_log && $chk_signoff && $signoff == 0) { ERROR("MISSING_SIGN_OFF", "Missing Signed-off-by: line(s)\n"); } -- cgit v1.2.3 From 45107ff6d5265b9786c62b694140d839bc3d2433 Mon Sep 17 00:00:00 2001 From: Allen Hubbe <allenbh@gmail.com> Date: Tue, 2 Aug 2016 14:04:47 -0700 Subject: checkpatch: if no filenames then read stdin If no filenames are given, then read the patch from stdin. Link: http://lkml.kernel.org/r/a8784f291ccb5067361992bf5d41ff6cfb0ce5cb.1469830917.git.allenbh@gmail.com Signed-off-by: Allen Hubbe <allenbh@gmail.com> Acked-by: Joe Perches <joe@perches.com> Cc: Andy Whitcroft <apw@canonical.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> --- scripts/checkpatch.pl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 6f2ce0cafe6f..4de3cc42fc50 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -228,9 +228,9 @@ if ($^V && $^V lt $minimum_perl_version) { } } +#if no filenames are given, push '-' to read patch from stdin if ($#ARGV < 0) { - print "$P: no input files\n"; - exit(1); + push(@ARGV, '-'); } sub hash_save_array_words { -- cgit v1.2.3 From 7e932159901183283cd82d797bc9a7c681e48e9c Mon Sep 17 00:00:00 2001 From: Joe Perches <joe@perches.com> Date: Thu, 1 Sep 2016 16:15:01 -0700 Subject: treewide: remove references to the now unnecessary DEFINE_PCI_DEVICE_TABLE It's been eliminated from the sources, remove it from everywhere else. Link: http://lkml.kernel.org/r/076eff466fd7edb550c25c8b25d76924ca0eba62.1472660229.git.joe@perches.com Signed-off-by: Joe Perches <joe@perches.com> Cc: "James E.J. Bottomley" <jejb@linux.vnet.ibm.com> Cc: "Martin K. Petersen" <martin.petersen@oracle.com> Cc: Bjorn Helgaas <bhelgaas@google.com> Cc: Andy Whitcroft <apw@canonical.com> Cc: Jonathan Corbet <corbet@lwn.net> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> --- Documentation/PCI/pci.txt | 1 - include/linux/pci.h | 9 --------- scripts/checkpatch.pl | 9 --------- scripts/tags.sh | 1 - 4 files changed, 20 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/Documentation/PCI/pci.txt b/Documentation/PCI/pci.txt index 123881f62219..77f49dc5be23 100644 --- a/Documentation/PCI/pci.txt +++ b/Documentation/PCI/pci.txt @@ -124,7 +124,6 @@ initialization with a pointer to a structure describing the driver The ID table is an array of struct pci_device_id entries ending with an all-zero entry. Definitions with static const are generally preferred. -Use of the deprecated macro DEFINE_PCI_DEVICE_TABLE should be avoided. Each entry consists of: diff --git a/include/linux/pci.h b/include/linux/pci.h index fbc1fa625c3e..0ab835965669 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -682,15 +682,6 @@ struct pci_driver { #define to_pci_driver(drv) container_of(drv, struct pci_driver, driver) -/** - * DEFINE_PCI_DEVICE_TABLE - macro used to describe a pci device table - * @_table: device table name - * - * This macro is deprecated and should not be used in new code. - */ -#define DEFINE_PCI_DEVICE_TABLE(_table) \ - const struct pci_device_id _table[] - /** * PCI_DEVICE - macro used to describe a specific pci device * @vend: the 16 bit PCI Vendor ID diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 4de3cc42fc50..206a6b346a8d 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -3570,15 +3570,6 @@ sub process { } } -# check for uses of DEFINE_PCI_DEVICE_TABLE - if ($line =~ /\bDEFINE_PCI_DEVICE_TABLE\s*\(\s*(\w+)\s*\)\s*=/) { - if (WARN("DEFINE_PCI_DEVICE_TABLE", - "Prefer struct pci_device_id over deprecated DEFINE_PCI_DEVICE_TABLE\n" . $herecurr) && - $fix) { - $fixed[$fixlinenr] =~ s/\b(?:static\s+|)DEFINE_PCI_DEVICE_TABLE\s*\(\s*(\w+)\s*\)\s*=\s*/static const struct pci_device_id $1\[\] = /; - } - } - # check for new typedefs, only function parameters and sparse annotations # make sense. if ($line =~ /\btypedef\s/ && diff --git a/scripts/tags.sh b/scripts/tags.sh index ed7eef24ef89..b3775a9604ea 100755 --- a/scripts/tags.sh +++ b/scripts/tags.sh @@ -206,7 +206,6 @@ regex_c=( '/\<DEFINE_PER_CPU_SHARED_ALIGNED([^,]*, *\([[:alnum:]_]*\)/\1/v/' '/\<DECLARE_WAIT_QUEUE_HEAD(\([[:alnum:]_]*\)/\1/v/' '/\<DECLARE_\(TASKLET\|WORK\|DELAYED_WORK\)(\([[:alnum:]_]*\)/\2/v/' - '/\<DEFINE_PCI_DEVICE_TABLE(\([[:alnum:]_]*\)/\1/v/' '/\(^\s\)OFFSET(\([[:alnum:]_]*\)/\2/v/' '/\(^\s\)DEFINE(\([[:alnum:]_]*\)/\2/v/' '/\<DEFINE_HASHTABLE(\([[:alnum:]_]*\)/\1/v/' -- cgit v1.2.3 From 85b0ee18bbf82cb3e1880c718749149c6ed61058 Mon Sep 17 00:00:00 2001 From: Joe Perches <joe@perches.com> Date: Tue, 11 Oct 2016 13:51:44 -0700 Subject: checkpatch: see if modified files are marked obsolete in MAINTAINERS Use get_maintainer to check the status of individual files. If "obsolete", suggest leaving the files alone. Link: http://lkml.kernel.org/r/7ceaa510dc9d2df05ec4b456baed7bb1415550b3.1471889575.git.joe@perches.com Signed-off-by: Joe Perches <joe@perches.com> Cc: SF Markus Elfring <elfring@users.sourceforge.net> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> --- scripts/checkpatch.pl | 14 ++++++++++++++ 1 file changed, 14 insertions(+) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 206a6b346a8d..34b445574011 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -704,6 +704,16 @@ sub seed_camelcase_file { } } +sub is_maintained_obsolete { + my ($filename) = @_; + + return 0 if (!(-e "$root/scripts/get_maintainer.pl")); + + my $status = `perl $root/scripts/get_maintainer.pl --status --nom --nol --nogit --nogit-fallback $filename 2>&1`; + + return $status =~ /obsolete/i; +} + my $camelcase_seeded = 0; sub seed_camelcase_includes { return if ($camelcase_seeded); @@ -2289,6 +2299,10 @@ sub process { } if ($found_file) { + if (is_maintained_obsolete($realfile)) { + WARN("OBSOLETE", + "$realfile is marked as 'obsolete' in the MAINTAINERS hierarchy. No unnecessary modifications please.\n"); + } if ($realfile =~ m@^(?:drivers/net/|net/|drivers/staging/)@) { $check = 1; } else { -- cgit v1.2.3 From f90774e1fd2700de4a6e0d62866d34a26c544bd0 Mon Sep 17 00:00:00 2001 From: Joe Perches <joe@perches.com> Date: Tue, 11 Oct 2016 13:51:47 -0700 Subject: checkpatch: look for symbolic permissions and suggest octal instead S_<FOO> uses should be avoided where octal is more intelligible. Linus didst say: : It's *much* easier to parse and understand the octal numbers, while the : symbolic macro names are just random line noise and hard as hell to : understand. You really have to think about it. : : So we should rather go the other way: convert existing bad symbolic : permission bit macro use to just use the octal numbers. : : The symbolic names are good for the *other* bits (ie sticky bit, and the : inode mode _type_ numbers etc), but for the permission bits, the symbolic : names are just insane crap. Nobody sane should ever use them. Not in the : kernel, not in user space. (http://lkml.kernel.org/r/CA+55aFw5v23T-zvDZp-MmD_EYxF8WbafwwB59934FV7g21uMGQ@mail.gmail.com) Link: http://lkml.kernel.org/r/7232ef011d05a92f4caa86a5e9830d87966a2eaf.1470180926.git.joe@perches.com Signed-off-by: Joe Perches <joe@perches.com> Cc: Linus Torvalds <torvalds@linux-foundation.org> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> --- scripts/checkpatch.pl | 49 +++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 43 insertions(+), 6 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 34b445574011..1c82b01d581a 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -541,6 +541,32 @@ our $mode_perms_world_writable = qr{ 0[0-7][0-7][2367] }x; +our %mode_permission_string_types = ( + "S_IRWXU" => 0700, + "S_IRUSR" => 0400, + "S_IWUSR" => 0200, + "S_IXUSR" => 0100, + "S_IRWXG" => 0070, + "S_IRGRP" => 0040, + "S_IWGRP" => 0020, + "S_IXGRP" => 0010, + "S_IRWXO" => 0007, + "S_IROTH" => 0004, + "S_IWOTH" => 0002, + "S_IXOTH" => 0001, + "S_IRWXUGO" => 0777, + "S_IRUGO" => 0444, + "S_IWUGO" => 0222, + "S_IXUGO" => 0111, +); + +#Create a search pattern for all these strings to speed up a loop below +our $mode_perms_string_search = ""; +foreach my $entry (keys %mode_permission_string_types) { + $mode_perms_string_search .= '|' if ($mode_perms_string_search ne ""); + $mode_perms_string_search .= $entry; +} + our $allowed_asm_includes = qr{(?x: irq| memory| @@ -6003,20 +6029,31 @@ sub process { $arg_pos--; $skip_args = "(?:\\s*$FuncArg\\s*,\\s*){$arg_pos,$arg_pos}"; } - my $test = "\\b$func\\s*\\(${skip_args}([\\d]+)\\s*[,\\)]"; + my $test = "\\b$func\\s*\\(${skip_args}($FuncArg(?:\\|\\s*$FuncArg)*)\\s*[,\\)]"; if ($line =~ /$test/) { my $val = $1; $val = $6 if ($skip_args ne ""); - - if ($val !~ /^0$/ && - (($val =~ /^$Int$/ && $val !~ /^$Octal$/) || - length($val) ne 4)) { + if (($val =~ /^$Int$/ && $val !~ /^$Octal$/) || + ($val =~ /^$Octal$/ && length($val) ne 4)) { ERROR("NON_OCTAL_PERMISSIONS", "Use 4 digit octal (0777) not decimal permissions\n" . $herecurr); - } elsif ($val =~ /^$Octal$/ && (oct($val) & 02)) { + } + if ($val =~ /^$Octal$/ && (oct($val) & 02)) { ERROR("EXPORTED_WORLD_WRITABLE", "Exporting writable files is usually an error. Consider more restrictive permissions.\n" . $herecurr); } + if ($val =~ /\b$mode_perms_string_search\b/) { + my $to = 0; + while ($val =~ /\b($mode_perms_string_search)\b(?:\s*\|\s*)?\s*/g) { + $to |= $mode_permission_string_types{$1}; + } + my $new = sprintf("%04o", $to); + if (WARN("SYMBOLIC_PERMS", + "Symbolic permissions are not preferred. Consider using octal permissions $new.\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/\Q$val\E/$new/; + } + } } } } -- cgit v1.2.3 From 08eb9b8016b18f53fa8900f3d1c9359f9edeb724 Mon Sep 17 00:00:00 2001 From: Joe Perches <joe@perches.com> Date: Tue, 11 Oct 2016 13:51:50 -0700 Subject: checkpatch: test multiple line block comment alignment Warn when block comments are not aligned on the * /* * block comment, no warning */ /* * block comment, emit warning */ Link: http://lkml.kernel.org/r/edb57bd330adfe024b95ec2a807d4aa7f0c8b112.1472261299.git.joe@perches.com Signed-off-by: Joe Perches <joe@perches.com> Reported-by: Sudip Mukherjee <sudipm.mukherjee@gmail.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> --- scripts/checkpatch.pl | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 1c82b01d581a..894f8c616573 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2979,6 +2979,25 @@ sub process { "Block comments use a trailing */ on a separate line\n" . $herecurr); } +# Block comment * alignment + if ($prevline =~ /$;[ \t]*$/ && #ends in comment + (($prevrawline =~ /^\+.*?\/\*/ && #starting /* + $prevrawline !~ /\*\/[ \t]*$/) || #no trailing */ + $prevrawline =~ /^\+[ \t]*\*/) && #starting * + $rawline =~ /^\+[ \t]*\*/) { #rawline * + $prevrawline =~ m@^\+([ \t]*/?)\*@; + my $oldindent = expand_tabs($1); + $rawline =~ m@^\+([ \t]*)\*@; + my $newindent = $1; + my $test_comment = '^\\+' . "$;" x (length($newindent) + 1); + $newindent = expand_tabs($newindent); + if ($line =~ /$test_comment/ && + length($oldindent) ne length($newindent)) { + WARN("BLOCK_COMMENT_STYLE", + "Block comments should align the * on each line\n" . $hereprev); + } + } + # check for missing blank lines after struct/union declarations # with exceptions for various attributes and macros if ($prevline =~ /^[\+ ]};?\s*$/ && -- cgit v1.2.3 From f333195d41e15e7b105ca270c7662204a1a9d0f5 Mon Sep 17 00:00:00 2001 From: Joe Perches <joe@perches.com> Date: Tue, 11 Oct 2016 13:51:53 -0700 Subject: checkpatch: don't test for prefer ether_addr_<foo> < sigh > Comment these tests out. These are just too enticing to people that don't verify that both source and dest addresses really must be __aligned(2). It helps make Dan Carpenter happy too. Link: http://lkml.kernel.org/r/dc32ec66d24647f4cdf824c8dfbbc59aa7ce7b7d.1472665676.git.joe@perches.com Signed-off-by: Joe Perches <joe@perches.com> Cc: Dan Carpenter <dan.carpenter@oracle.com> Cc: Greg <gvrose8192@gmail.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> --- scripts/checkpatch.pl | 70 +++++++++++++++++++++++++-------------------------- 1 file changed, 35 insertions(+), 35 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 894f8c616573..37d8b91ac198 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -5554,46 +5554,46 @@ sub process { } # Check for memcpy(foo, bar, ETH_ALEN) that could be ether_addr_copy(foo, bar) - if ($^V && $^V ge 5.10.0 && - defined $stat && - $stat =~ /^\+(?:.*?)\bmemcpy\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\,\s*ETH_ALEN\s*\)/) { - if (WARN("PREFER_ETHER_ADDR_COPY", - "Prefer ether_addr_copy() over memcpy() if the Ethernet addresses are __aligned(2)\n" . "$here\n$stat\n") && - $fix) { - $fixed[$fixlinenr] =~ s/\bmemcpy\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\,\s*ETH_ALEN\s*\)/ether_addr_copy($2, $7)/; - } - } +# if ($^V && $^V ge 5.10.0 && +# defined $stat && +# $stat =~ /^\+(?:.*?)\bmemcpy\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\,\s*ETH_ALEN\s*\)/) { +# if (WARN("PREFER_ETHER_ADDR_COPY", +# "Prefer ether_addr_copy() over memcpy() if the Ethernet addresses are __aligned(2)\n" . "$here\n$stat\n") && +# $fix) { +# $fixed[$fixlinenr] =~ s/\bmemcpy\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\,\s*ETH_ALEN\s*\)/ether_addr_copy($2, $7)/; +# } +# } # Check for memcmp(foo, bar, ETH_ALEN) that could be ether_addr_equal*(foo, bar) - if ($^V && $^V ge 5.10.0 && - defined $stat && - $stat =~ /^\+(?:.*?)\bmemcmp\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\,\s*ETH_ALEN\s*\)/) { - WARN("PREFER_ETHER_ADDR_EQUAL", - "Prefer ether_addr_equal() or ether_addr_equal_unaligned() over memcmp()\n" . "$here\n$stat\n") - } +# if ($^V && $^V ge 5.10.0 && +# defined $stat && +# $stat =~ /^\+(?:.*?)\bmemcmp\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\,\s*ETH_ALEN\s*\)/) { +# WARN("PREFER_ETHER_ADDR_EQUAL", +# "Prefer ether_addr_equal() or ether_addr_equal_unaligned() over memcmp()\n" . "$here\n$stat\n") +# } # check for memset(foo, 0x0, ETH_ALEN) that could be eth_zero_addr # check for memset(foo, 0xFF, ETH_ALEN) that could be eth_broadcast_addr - if ($^V && $^V ge 5.10.0 && - defined $stat && - $stat =~ /^\+(?:.*?)\bmemset\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\,\s*ETH_ALEN\s*\)/) { - - my $ms_val = $7; - - if ($ms_val =~ /^(?:0x|)0+$/i) { - if (WARN("PREFER_ETH_ZERO_ADDR", - "Prefer eth_zero_addr over memset()\n" . "$here\n$stat\n") && - $fix) { - $fixed[$fixlinenr] =~ s/\bmemset\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*,\s*ETH_ALEN\s*\)/eth_zero_addr($2)/; - } - } elsif ($ms_val =~ /^(?:0xff|255)$/i) { - if (WARN("PREFER_ETH_BROADCAST_ADDR", - "Prefer eth_broadcast_addr() over memset()\n" . "$here\n$stat\n") && - $fix) { - $fixed[$fixlinenr] =~ s/\bmemset\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*,\s*ETH_ALEN\s*\)/eth_broadcast_addr($2)/; - } - } - } +# if ($^V && $^V ge 5.10.0 && +# defined $stat && +# $stat =~ /^\+(?:.*?)\bmemset\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\,\s*ETH_ALEN\s*\)/) { +# +# my $ms_val = $7; +# +# if ($ms_val =~ /^(?:0x|)0+$/i) { +# if (WARN("PREFER_ETH_ZERO_ADDR", +# "Prefer eth_zero_addr over memset()\n" . "$here\n$stat\n") && +# $fix) { +# $fixed[$fixlinenr] =~ s/\bmemset\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*,\s*ETH_ALEN\s*\)/eth_zero_addr($2)/; +# } +# } elsif ($ms_val =~ /^(?:0xff|255)$/i) { +# if (WARN("PREFER_ETH_BROADCAST_ADDR", +# "Prefer eth_broadcast_addr() over memset()\n" . "$here\n$stat\n") && +# $fix) { +# $fixed[$fixlinenr] =~ s/\bmemset\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*,\s*ETH_ALEN\s*\)/eth_broadcast_addr($2)/; +# } +# } +# } # typecasts on min/max could be min_t/max_t if ($^V && $^V ge 5.10.0 && -- cgit v1.2.3 From bf1fa1dae68e1b58b7b7fd61bde654d37da1de57 Mon Sep 17 00:00:00 2001 From: Joe Perches <joe@perches.com> Date: Tue, 11 Oct 2016 13:51:56 -0700 Subject: checkpatch: externalize the structs that should be const Make it easier to add new structs that should be const. Link: http://lkml.kernel.org/r/e5a8da43e7c11525bafbda1ca69a8323614dd942.1472664220.git.joe@perches.com Signed-off-by: Joe Perches <joe@perches.com> Cc: Julia Lawall <julia.lawall@lip6.fr> Cc: Kees Cook <keescook@chromium.org> Cc: Andy Whitcroft <apw@canonical.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> --- scripts/checkpatch.pl | 64 +++++++++++++++------------------------- scripts/const_structs.checkpatch | 39 ++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 40 deletions(-) create mode 100644 scripts/const_structs.checkpatch (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 37d8b91ac198..4ecb66c595f4 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -54,6 +54,7 @@ my $min_conf_desc_length = 4; my $spelling_file = "$D/spelling.txt"; my $codespell = 0; my $codespellfile = "/usr/share/codespell/dictionary.txt"; +my $conststructsfile = "$D/const_structs.checkpatch"; my $color = 1; my $allow_c99_comments = 1; @@ -624,6 +625,29 @@ if ($codespell) { $misspellings = join("|", sort keys %spelling_fix) if keys %spelling_fix; +my $const_structs = ""; +if (open(my $conststructs, '<', $conststructsfile)) { + while (<$conststructs>) { + my $line = $_; + + $line =~ s/\s*\n?$//g; + $line =~ s/^\s*//g; + + next if ($line =~ m/^\s*#/); + next if ($line =~ m/^\s*$/); + if ($line =~ /\s/) { + print("$conststructsfile: '$line' invalid - ignored\n"); + next; + } + + $const_structs .= '|' if ($const_structs ne ""); + $const_structs .= $line; + } + close($conststructsfile); +} else { + warn "No structs that should be const will be found - file '$conststructsfile': $!\n"; +} + sub build_types { my $mods = "(?x: \n" . join("|\n ", (@modifierList, @modifierListFile)) . "\n)"; my $all = "(?x: \n" . join("|\n ", (@typeList, @typeListFile)) . "\n)"; @@ -5912,46 +5936,6 @@ sub process { } # check for various structs that are normally const (ops, kgdb, device_tree) - my $const_structs = qr{ - acpi_dock_ops| - address_space_operations| - backlight_ops| - block_device_operations| - dentry_operations| - dev_pm_ops| - dma_map_ops| - extent_io_ops| - file_lock_operations| - file_operations| - hv_ops| - ide_dma_ops| - intel_dvo_dev_ops| - item_operations| - iwl_ops| - kgdb_arch| - kgdb_io| - kset_uevent_ops| - lock_manager_operations| - microcode_ops| - mtrr_ops| - neigh_ops| - nlmsvc_binding| - of_device_id| - pci_raw_ops| - pipe_buf_operations| - platform_hibernation_ops| - platform_suspend_ops| - proto_ops| - rpc_pipe_ops| - seq_operations| - snd_ac97_build_ops| - soc_pcmcia_socket_ops| - stacktrace_ops| - sysfs_ops| - tty_operations| - uart_ops| - usb_mon_operations| - wd_ops}x; if ($line !~ /\bconst\b/ && $line =~ /\bstruct\s+($const_structs)\b/) { WARN("CONST_STRUCT", diff --git a/scripts/const_structs.checkpatch b/scripts/const_structs.checkpatch new file mode 100644 index 000000000000..1b54425f6c89 --- /dev/null +++ b/scripts/const_structs.checkpatch @@ -0,0 +1,39 @@ +acpi_dock_ops +address_space_operations +backlight_ops +block_device_operations +dentry_operations +dev_pm_ops +dma_map_ops +extent_io_ops +file_lock_operations +file_operations +hv_ops +ide_dma_ops +intel_dvo_dev_ops +item_operations +iwl_ops +kgdb_arch +kgdb_io +kset_uevent_ops +lock_manager_operations +microcode_ops +mtrr_ops +neigh_ops +nlmsvc_binding +of_device_id +pci_raw_ops +pipe_buf_operations +platform_hibernation_ops +platform_suspend_ops +proto_ops +rpc_pipe_ops +seq_operations +snd_ac97_build_ops +soc_pcmcia_socket_ops +stacktrace_ops +sysfs_ops +tty_operations +uart_ops +usb_mon_operations +wd_ops -- cgit v1.2.3 From 0616efa45af493a259d25c1716102173f10bd789 Mon Sep 17 00:00:00 2001 From: Joe Perches <joe@perches.com> Date: Tue, 11 Oct 2016 13:52:02 -0700 Subject: checkpatch: speed up checking for filenames in sections marked obsolete Adding -f to the get_maintainer.pl invocation means git isn't invoked by get_maintainer.pl for known filenames. This reduces the overall time to run checkpatch. Link: http://lkml.kernel.org/r/22991e3a295aeb399b43af0478b6e5809106ccee.1472684066.git.joe@perches.com Signed-off-by: Joe Perches <joe@perches.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> --- scripts/checkpatch.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 4ecb66c595f4..e44dc0c75d54 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -759,7 +759,7 @@ sub is_maintained_obsolete { return 0 if (!(-e "$root/scripts/get_maintainer.pl")); - my $status = `perl $root/scripts/get_maintainer.pl --status --nom --nol --nogit --nogit-fallback $filename 2>&1`; + my $status = `perl $root/scripts/get_maintainer.pl --status --nom --nol --nogit --nogit-fallback -f $filename 2>&1`; return $status =~ /obsolete/i; } -- cgit v1.2.3 From af207524a49c3d70af0a9196072ab3b6d690fcf7 Mon Sep 17 00:00:00 2001 From: Joe Perches <joe@perches.com> Date: Tue, 11 Oct 2016 13:52:05 -0700 Subject: checkpatch: improve the block comment * alignment test An "uninitialized value" is emitted when a block comment starts on the same line as a statement. Fix this and make the test use a little fewer cpu cycles too. Link: http://lkml.kernel.org/r/3c9993320c2182d37f53ac540878cfef59c3f62d.1473365956.git.joe@perches.com Signed-off-by: Joe Perches <joe@perches.com> Reported-by: Charlemagne Lasse <charlemagnelasse@gmail.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> --- scripts/checkpatch.pl | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index e44dc0c75d54..3fa2b2e973eb 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -3005,18 +3005,23 @@ sub process { # Block comment * alignment if ($prevline =~ /$;[ \t]*$/ && #ends in comment - (($prevrawline =~ /^\+.*?\/\*/ && #starting /* + $line =~ /^\+[ \t]*$;/ && #leading comment + $rawline =~ /^\+[ \t]*\*/ && #leading * + (($prevrawline =~ /^\+.*?\/\*/ && #leading /* $prevrawline !~ /\*\/[ \t]*$/) || #no trailing */ - $prevrawline =~ /^\+[ \t]*\*/) && #starting * - $rawline =~ /^\+[ \t]*\*/) { #rawline * + $prevrawline =~ /^\+[ \t]*\*/)) { #leading * + my $oldindent; $prevrawline =~ m@^\+([ \t]*/?)\*@; - my $oldindent = expand_tabs($1); + if (defined($1)) { + $oldindent = expand_tabs($1); + } else { + $prevrawline =~ m@^\+(.*/?)\*@; + $oldindent = expand_tabs($1); + } $rawline =~ m@^\+([ \t]*)\*@; my $newindent = $1; - my $test_comment = '^\\+' . "$;" x (length($newindent) + 1); $newindent = expand_tabs($newindent); - if ($line =~ /$test_comment/ && - length($oldindent) ne length($newindent)) { + if (length($oldindent) ne length($newindent)) { WARN("BLOCK_COMMENT_STYLE", "Block comments should align the * on each line\n" . $hereprev); } -- cgit v1.2.3 From f59b64bffe163431b4ec61f3b00c1aeb846948f3 Mon Sep 17 00:00:00 2001 From: Joe Perches <joe@perches.com> Date: Tue, 11 Oct 2016 13:52:08 -0700 Subject: checkpatch: add --strict test for macro argument reuse If a macro argument is used multiple times in the macro definition, the macro argument may have an unexpected side-effect. Add a test (MACRO_ARG_REUSE) for that condition which is only emitted with command-line option --strict. Link: http://lkml.kernel.org/r/b6d67a87cafcafd15499e91780dc63b15dec0aa0.1473744906.git.joe@perches.com Signed-off-by: Joe Perches <joe@perches.com> Cc: Andy Whitcroft <apw@canonical.com> Cc: Julia Lawall <julia.lawall@lip6.fr> Cc: Dan Carpenter <dan.carpenter@oracle.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> --- scripts/checkpatch.pl | 43 +++++++++++++++++++++++++++++++++++-------- 1 file changed, 35 insertions(+), 8 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 3fa2b2e973eb..f135f8e4d069 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -4753,7 +4753,17 @@ sub process { $has_flow_statement = 1 if ($ctx =~ /\b(goto|return)\b/); $has_arg_concat = 1 if ($ctx =~ /\#\#/ && $ctx !~ /\#\#\s*(?:__VA_ARGS__|args)\b/); - $dstat =~ s/^.\s*\#\s*define\s+$Ident(?:\([^\)]*\))?\s*//; + $dstat =~ s/^.\s*\#\s*define\s+$Ident(\([^\)]*\))?\s*//; + my $define_args = $1; + my $define_stmt = $dstat; + my @def_args = (); + + if (defined $define_args && $define_args ne "") { + $define_args = substr($define_args, 1, length($define_args) - 2); + $define_args =~ s/\s*//g; + @def_args = split(",", $define_args); + } + $dstat =~ s/$;//g; $dstat =~ s/\\\n.//g; $dstat =~ s/^\s*//s; @@ -4789,6 +4799,15 @@ sub process { ^\[ }x; #print "REST<$rest> dstat<$dstat> ctx<$ctx>\n"; + + $ctx =~ s/\n*$//; + my $herectx = $here . "\n"; + my $stmt_cnt = statement_rawlines($ctx); + + for (my $n = 0; $n < $stmt_cnt; $n++) { + $herectx .= raw_line($linenr, $n) . "\n"; + } + if ($dstat ne '' && $dstat !~ /^(?:$Ident|-?$Constant),$/ && # 10, // foo(), $dstat !~ /^(?:$Ident|-?$Constant);$/ && # foo(); @@ -4804,13 +4823,6 @@ sub process { $dstat !~ /^\(\{/ && # ({... $ctx !~ /^.\s*#\s*define\s+TRACE_(?:SYSTEM|INCLUDE_FILE|INCLUDE_PATH)\b/) { - $ctx =~ s/\n*$//; - my $herectx = $here . "\n"; - my $cnt = statement_rawlines($ctx); - - for (my $n = 0; $n < $cnt; $n++) { - $herectx .= raw_line($linenr, $n) . "\n"; - } if ($dstat =~ /;/) { ERROR("MULTISTATEMENT_MACRO_USE_DO_WHILE", @@ -4819,6 +4831,21 @@ sub process { ERROR("COMPLEX_MACRO", "Macros with complex values should be enclosed in parentheses\n" . "$herectx"); } + + } +# check if any macro arguments are reused (ignore '...' and 'type') + foreach my $arg (@def_args) { + next if ($arg =~ /\.\.\./); + next if ($arg =~ /^type$/); + my $tmp = $define_stmt; + $tmp =~ s/\b(typeof|__typeof__|__builtin\w+|typecheck\s*\(\s*$Type\s*,|\#+)\s*\(*\s*$arg\s*\)*\b//g; + $tmp =~ s/\#\s*$arg\b//g; + $tmp =~ s/\b$arg\s*\#\#//g; + my $use_cnt = $tmp =~ s/\b$arg\b//g; + if ($use_cnt > 1) { + CHK("MACRO_ARG_REUSE", + "Macro argument reuse '$arg' - possible side-effects?\n" . "$herectx"); + } } # check for macros with flow control, but without ## concatenation -- cgit v1.2.3 From 9192d41a3f0b98844860f1de5b82f729bbe07c61 Mon Sep 17 00:00:00 2001 From: Joe Perches <joe@perches.com> Date: Tue, 11 Oct 2016 13:52:11 -0700 Subject: checkpatch: add --strict test for precedence challenged macro arguments Add a test for macro arguents that have a non-comma leading or trailing operator where the argument isn't parenthesized to avoid possible precedence issues. Link: http://lkml.kernel.org/r/47715508972f8d786f435e583ff881dbeee3a114.1473745855.git.joe@perches.com Signed-off-by: Joe Perches <joe@perches.com> Cc: Andy Whitcroft <apw@canonical.com> Cc: Julia Lawall <julia.lawall@lip6.fr> Cc: Dan Carpenter <dan.carpenter@oracle.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> --- scripts/checkpatch.pl | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index f135f8e4d069..ea1a7adce406 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -4836,7 +4836,7 @@ sub process { # check if any macro arguments are reused (ignore '...' and 'type') foreach my $arg (@def_args) { next if ($arg =~ /\.\.\./); - next if ($arg =~ /^type$/); + next if ($arg =~ /^type$/i); my $tmp = $define_stmt; $tmp =~ s/\b(typeof|__typeof__|__builtin\w+|typecheck\s*\(\s*$Type\s*,|\#+)\s*\(*\s*$arg\s*\)*\b//g; $tmp =~ s/\#\s*$arg\b//g; @@ -4845,6 +4845,13 @@ sub process { if ($use_cnt > 1) { CHK("MACRO_ARG_REUSE", "Macro argument reuse '$arg' - possible side-effects?\n" . "$herectx"); + } +# check if any macro arguments may have other precedence issues + if ($define_stmt =~ m/($Operators)?\s*\b$arg\b\s*($Operators)?/m && + ((defined($1) && $1 ne ',') || + (defined($2) && $2 ne ','))) { + CHK("MACRO_ARG_PRECEDENCE", + "Macro argument '$arg' may be better as '($arg)' to avoid precedence issues\n" . "$herectx"); } } -- cgit v1.2.3 From 5207649b7b1d2b1e1f8c97df91debfaa8de37669 Mon Sep 17 00:00:00 2001 From: Joe Perches <joe@perches.com> Date: Tue, 11 Oct 2016 13:52:14 -0700 Subject: checkpatch: improve MACRO_ARG_PRECEDENCE test It is possible for a multiple line macro definition to have a false positive report when an argument is used on a line after a continuation \. This line might have a leading '+' as the initial character that could be confused by checkpatch as an operator. Avoid the leading character on multiple line macro definitions. Link: http://lkml.kernel.org/r/60229d13399f9b6509db5a32e30d4c16951a60cd.1473836073.git.joe@perches.com Signed-off-by: Joe Perches <joe@perches.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> --- scripts/checkpatch.pl | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index ea1a7adce406..0ef3d837f2aa 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -4833,13 +4833,31 @@ sub process { } } + + # Make $define_stmt single line, comment-free, etc + my @stmt_array = split('\n', $define_stmt); + my $first = 1; + $define_stmt = ""; + foreach my $l (@stmt_array) { + $l =~ s/\\$//; + if ($first) { + $define_stmt = $l; + $first = 0; + } elsif ($l =~ /^[\+ ]/) { + $define_stmt .= substr($l, 1); + } + } + $define_stmt =~ s/$;//g; + $define_stmt =~ s/\s+/ /g; + $define_stmt = trim($define_stmt); + # check if any macro arguments are reused (ignore '...' and 'type') foreach my $arg (@def_args) { next if ($arg =~ /\.\.\./); next if ($arg =~ /^type$/i); my $tmp = $define_stmt; $tmp =~ s/\b(typeof|__typeof__|__builtin\w+|typecheck\s*\(\s*$Type\s*,|\#+)\s*\(*\s*$arg\s*\)*\b//g; - $tmp =~ s/\#\s*$arg\b//g; + $tmp =~ s/\#+\s*$arg\b//g; $tmp =~ s/\b$arg\s*\#\#//g; my $use_cnt = $tmp =~ s/\b$arg\b//g; if ($use_cnt > 1) { -- cgit v1.2.3 From ca0d8929e75ab1f860f61208d46955f280a1b05e Mon Sep 17 00:00:00 2001 From: Joe Perches <joe@perches.com> Date: Tue, 11 Oct 2016 13:52:16 -0700 Subject: checkpatch: add warning for unnamed function definition arguments Function definitions without identifiers like int foo(int) are not preferred. Emit a warning when they occur. Link: http://lkml.kernel.org/r/94fe6378504745991b650f48fc92bb4648f25706.1474925354.git.joe@perches.com Signed-off-by: Joe Perches <joe@perches.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> --- scripts/checkpatch.pl | 13 +++++++++++++ 1 file changed, 13 insertions(+) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 0ef3d837f2aa..3373c65fef1c 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -5794,6 +5794,19 @@ sub process { "externs should be avoided in .c files\n" . $herecurr); } + if ($realfile =~ /\.[ch]$/ && defined $stat && + $stat =~ /^.\s*(?:extern\s+)?$Type\s*$Ident\s*\(\s*([^{]+)\s*\)\s*;/s && + $1 ne "void") { + my $args = trim($1); + while ($args =~ m/\s*($Type\s*(?:$Ident|\(\s*\*\s*$Ident?\s*\)\s*$balanced_parens)?)/g) { + my $arg = trim($1); + if ($arg =~ /^$Type$/ && $arg !~ /enum\s+$Ident$/) { + WARN("FUNCTION_ARGUMENTS", + "function definition argument '$arg' should also have an identifier name\n" . $herecurr); + } + } + } + # checks for new __setup's if ($rawline =~ /\b__setup\("([^"]*)"/) { my $name = $1; -- cgit v1.2.3 From 459cf0ae5d6600592a539bc861c84e14ed1a5bcb Mon Sep 17 00:00:00 2001 From: Joe Perches <joe@perches.com> Date: Tue, 11 Oct 2016 13:52:19 -0700 Subject: checkpatch: improve the octal permissions tests The function calls with octal permissions commonly span multiple lines. The current test is line oriented and fails to find some matches. Make the test use the $stat variable instead of the $line variable to span multiple lines. Also add a few functions to the known functions with permissions list. Move the SYMBOLIC_PERMS test to a separate section to find all the S_<FOO> permissions in any form not just those that have specific function names. This can now find and fix permissions uses like: .mode = S_<FOO> | S_<BAR>; Link: http://lkml.kernel.org/r/b51bab60530912aae4ac420119d465c5b206f19f.1475030406.git.joe@perches.com Signed-off-by: Joe Perches <joe@perches.com> Tested-by: Ramiro Oliveira <roliveir@synopsys.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> --- scripts/checkpatch.pl | 60 +++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 44 insertions(+), 16 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 3373c65fef1c..a8368d1c4348 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -524,7 +524,11 @@ our @mode_permission_funcs = ( ["module_param_array_named", 5], ["debugfs_create_(?:file|u8|u16|u32|u64|x8|x16|x32|x64|size_t|atomic_t|bool|blob|regset32|u32_array)", 2], ["proc_create(?:_data|)", 2], - ["(?:CLASS|DEVICE|SENSOR)_ATTR", 2], + ["(?:CLASS|DEVICE|SENSOR|SENSOR_DEVICE|IIO_DEVICE)_ATTR", 2], + ["IIO_DEV_ATTR_[A-Z_]+", 1], + ["SENSOR_(?:DEVICE_|)ATTR_2", 2], + ["SENSOR_TEMPLATE(?:_2|)", 3], + ["__ATTR", 2], ); #Create a search pattern for all these functions to speed up a loop below @@ -6092,45 +6096,69 @@ sub process { # Mode permission misuses where it seems decimal should be octal # This uses a shortcut match to avoid unnecessary uses of a slow foreach loop if ($^V && $^V ge 5.10.0 && + defined $stat && $line =~ /$mode_perms_search/) { foreach my $entry (@mode_permission_funcs) { my $func = $entry->[0]; my $arg_pos = $entry->[1]; + my $lc = $stat =~ tr@\n@@; + $lc = $lc + $linenr; + my $stat_real = raw_line($linenr, 0); + for (my $count = $linenr + 1; $count <= $lc; $count++) { + $stat_real = $stat_real . "\n" . raw_line($count, 0); + } + my $skip_args = ""; if ($arg_pos > 1) { $arg_pos--; $skip_args = "(?:\\s*$FuncArg\\s*,\\s*){$arg_pos,$arg_pos}"; } my $test = "\\b$func\\s*\\(${skip_args}($FuncArg(?:\\|\\s*$FuncArg)*)\\s*[,\\)]"; - if ($line =~ /$test/) { + if ($stat =~ /$test/) { my $val = $1; $val = $6 if ($skip_args ne ""); if (($val =~ /^$Int$/ && $val !~ /^$Octal$/) || ($val =~ /^$Octal$/ && length($val) ne 4)) { ERROR("NON_OCTAL_PERMISSIONS", - "Use 4 digit octal (0777) not decimal permissions\n" . $herecurr); + "Use 4 digit octal (0777) not decimal permissions\n" . "$here\n" . $stat_real); } if ($val =~ /^$Octal$/ && (oct($val) & 02)) { ERROR("EXPORTED_WORLD_WRITABLE", - "Exporting writable files is usually an error. Consider more restrictive permissions.\n" . $herecurr); - } - if ($val =~ /\b$mode_perms_string_search\b/) { - my $to = 0; - while ($val =~ /\b($mode_perms_string_search)\b(?:\s*\|\s*)?\s*/g) { - $to |= $mode_permission_string_types{$1}; - } - my $new = sprintf("%04o", $to); - if (WARN("SYMBOLIC_PERMS", - "Symbolic permissions are not preferred. Consider using octal permissions $new.\n" . $herecurr) && - $fix) { - $fixed[$fixlinenr] =~ s/\Q$val\E/$new/; - } + "Exporting writable files is usually an error. Consider more restrictive permissions.\n" . "$here\n" . $stat_real); } } } } +# check for uses of S_<PERMS> that could be octal for readability + if ($line =~ /\b$mode_perms_string_search\b/) { + my $val = ""; + my $oval = ""; + my $to = 0; + my $curpos = 0; + my $lastpos = 0; + while ($line =~ /\b(($mode_perms_string_search)\b(?:\s*\|\s*)?\s*)/g) { + $curpos = pos($line); + my $match = $2; + my $omatch = $1; + last if ($lastpos > 0 && ($curpos - length($omatch) != $lastpos)); + $lastpos = $curpos; + $to |= $mode_permission_string_types{$match}; + $val .= '\s*\|\s*' if ($val ne ""); + $val .= $match; + $oval .= $omatch; + } + $oval =~ s/^\s*\|\s*//; + $oval =~ s/\s*\|\s*$//; + my $octal = sprintf("%04o", $to); + if (WARN("SYMBOLIC_PERMS", + "Symbolic permissions '$oval' are not preferred. Consider using octal permissions '$octal'.\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/$val/$octal/; + } + } + # validate content of MODULE_LICENSE against list from include/linux/module.h if ($line =~ /\bMODULE_LICENSE\s*\(\s*($String)\s*\)/) { my $extracted_string = get_quoted_string($line, $rawline); -- cgit v1.2.3