* [PATCH 2/2] Task 2: checkpath.pl documentation mentorship: remove semicolon warnings.
@ 2021-08-01 23:08 Rahul Balaji
2021-08-02 6:57 ` Lukas Bulwahn
0 siblings, 1 reply; 2+ messages in thread
From: Rahul Balaji @ 2021-08-01 23:08 UTC (permalink / raw)
To: lukas.bulwahn; +Cc: dwaipayanray1, linux-kernel-mentees
i changed the lines such that they no longer evoke the (ONE_SEMICOLON) warning
where possible.
there were 7 files with true positive violation.
drivers/mtd/ubi/block.c:408: WARNING:ONE_SEMICOLON: Statements
terminations use 1 semicolon
drivers/gpu/drm/nouveau/nouveau_gem.c:342: WARNING:ONE_SEMICOLON:
Statements terminations use 1 semicolon
fs/block_dev.c:1269: WARNING:ONE_SEMICOLON: Statements terminations
use 1 semicolon
(for fs/block_dev.c, the error was actually on line 1271 for me, not on 1269)
include/trace/events/iocost.h:163: WARNING:ONE_SEMICOLON: Statements
terminations use 1 semicolon
net/sched/cls_rsvp.h:231: WARNING:ONE_SEMICOLON: Statements
terminations use 1 semicolon
net/sched/cls_u32.c:782: WARNING:ONE_SEMICOLON: Statements
terminations use 1 semicolon
drivers/net/ethernet/mediatek/mtk_star_emac.c:1089:
WARNING:ONE_SEMICOLON: Statements terminations use 1 semicolon
the rest of the files given below i think are false positives, as they repeatedly use ";;" in their code.
so i guess it is necessary in their case.
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
---
drivers/gpu/drm/nouveau/nouveau_gem.c | 2 +-
drivers/net/ethernet/mediatek/mtk_star_emac.c | 2 +-
fs/block_dev.c | 2 +-
include/trace/events/iocost.h | 2 +-
net/sched/cls_rsvp.h | 2 +-
net/sched/cls_u32.c | 2 +-
6 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c b/drivers/gpu/drm/nouveau/nouveau_gem.c
index 5b27845075a1..d476940ee97c 100644
--- a/drivers/gpu/drm/nouveau/nouveau_gem.c
+++ b/drivers/gpu/drm/nouveau/nouveau_gem.c
@@ -339,7 +339,7 @@ nouveau_gem_set_domain(struct drm_gem_object *gem, uint32_t read_domains,
struct ttm_buffer_object *bo = &nvbo->bo;
uint32_t domains = valid_domains & nvbo->valid_domains &
(write_domains ? write_domains : read_domains);
- uint32_t pref_domains = 0;;
+ uint32_t pref_domains = 0;
if (!domains)
return -EINVAL;
diff --git a/drivers/net/ethernet/mediatek/mtk_star_emac.c b/drivers/net/ethernet/mediatek/mtk_star_emac.c
index 96d2891f1675..0a445dde67ae 100644
--- a/drivers/net/ethernet/mediatek/mtk_star_emac.c
+++ b/drivers/net/ethernet/mediatek/mtk_star_emac.c
@@ -1086,7 +1086,7 @@ static void mtk_star_tx_complete_all(struct mtk_star_priv *priv)
spin_lock(&priv->lock);
- for (pkts_compl = 0, bytes_compl = 0;;
+ for (pkts_compl = 0, bytes_compl = 0;
pkts_compl++, bytes_compl += ret, wake = true) {
if (!mtk_star_ring_descs_available(ring))
break;
diff --git a/fs/block_dev.c b/fs/block_dev.c
index 9ef4f1fc2cb0..61b938fa5d5b 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -1268,7 +1268,7 @@ static int blkdev_get_whole(struct block_device *bdev, fmode_t mode)
if (test_bit(GD_NEED_PART_SCAN, &disk->state))
bdev_disk_changed(disk, false);
bdev->bd_openers++;
- return 0;;
+ return 0;
}
static void blkdev_put_whole(struct block_device *bdev, fmode_t mode)
diff --git a/include/trace/events/iocost.h b/include/trace/events/iocost.h
index e282ce02fa2d..6d1626e7a4ce 100644
--- a/include/trace/events/iocost.h
+++ b/include/trace/events/iocost.h
@@ -160,7 +160,7 @@ TRACE_EVENT(iocost_ioc_vrate_adj,
TP_fast_assign(
__assign_str(devname, ioc_name(ioc));
- __entry->old_vrate = atomic64_read(&ioc->vtime_rate);;
+ __entry->old_vrate = atomic64_read(&ioc->vtime_rate);
__entry->new_vrate = new_vrate;
__entry->busy_level = ioc->busy_level;
__entry->read_missed_ppm = missed_ppm[READ];
diff --git a/net/sched/cls_rsvp.h b/net/sched/cls_rsvp.h
index 27a4b6dbcf57..b7929dfcca1f 100644
--- a/net/sched/cls_rsvp.h
+++ b/net/sched/cls_rsvp.h
@@ -228,7 +228,7 @@ static void rsvp_replace(struct tcf_proto *tp, struct rsvp_filter *n, u32 h)
for (s = rtnl_dereference(head->ht[h1]); s;
s = rtnl_dereference(s->next)) {
- for (ins = &s->ht[h2], pins = rtnl_dereference(*ins); ;
+ for (ins = &s->ht[h2], pins = rtnl_dereference(*ins);
ins = &pins->next, pins = rtnl_dereference(*ins)) {
if (pins->handle == h) {
RCU_INIT_POINTER(n->next, pins->next);
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index 6e1abe805448..290359f6ce6f 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -779,7 +779,7 @@ static void u32_replace_knode(struct tcf_proto *tp, struct tc_u_common *tp_c,
/* The node must always exist for it to be replaced if this is not the
* case then something went very wrong elsewhere.
*/
- for (pins = rtnl_dereference(*ins); ;
+ for (pins = rtnl_dereference(*ins);
ins = &pins->next, pins = rtnl_dereference(*ins))
if (pins->handle == n->handle)
break;
--
2.25.1
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH 2/2] Task 2: checkpath.pl documentation mentorship: remove semicolon warnings.
2021-08-01 23:08 [PATCH 2/2] Task 2: checkpath.pl documentation mentorship: remove semicolon warnings Rahul Balaji
@ 2021-08-02 6:57 ` Lukas Bulwahn
0 siblings, 0 replies; 2+ messages in thread
From: Lukas Bulwahn @ 2021-08-02 6:57 UTC (permalink / raw)
To: Rahul Balaji; +Cc: Dwaipayan Ray, linux-kernel-mentees
On Mon, Aug 2, 2021 at 1:09 AM Rahul Balaji <rahulbalaji78@gmail.com> wrote:
>
> i changed the lines such that they no longer evoke the (ONE_SEMICOLON) warning
> where possible.
>
> there were 7 files with true positive violation.
>
> drivers/mtd/ubi/block.c:408: WARNING:ONE_SEMICOLON: Statements
> terminations use 1 semicolon
>
> drivers/gpu/drm/nouveau/nouveau_gem.c:342: WARNING:ONE_SEMICOLON:
> Statements terminations use 1 semicolon
>
> fs/block_dev.c:1269: WARNING:ONE_SEMICOLON: Statements terminations
> use 1 semicolon
>
> (for fs/block_dev.c, the error was actually on line 1271 for me, not on 1269)
>
> include/trace/events/iocost.h:163: WARNING:ONE_SEMICOLON: Statements
> terminations use 1 semicolon
>
> net/sched/cls_rsvp.h:231: WARNING:ONE_SEMICOLON: Statements
> terminations use 1 semicolon
>
> net/sched/cls_u32.c:782: WARNING:ONE_SEMICOLON: Statements
> terminations use 1 semicolon
>
> drivers/net/ethernet/mediatek/mtk_star_emac.c:1089:
> WARNING:ONE_SEMICOLON: Statements terminations use 1 semicolon
>
> the rest of the files given below i think are false positives, as they repeatedly use ";;" in their code.
> so i guess it is necessary in their case.
Can you explain your guess? Why should the number of occurrences in
one file be an indicator for a true positive or false positive?
>
> 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
> ---
> drivers/gpu/drm/nouveau/nouveau_gem.c | 2 +-
> drivers/net/ethernet/mediatek/mtk_star_emac.c | 2 +-
> fs/block_dev.c | 2 +-
> include/trace/events/iocost.h | 2 +-
> net/sched/cls_rsvp.h | 2 +-
> net/sched/cls_u32.c | 2 +-
> 6 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c b/drivers/gpu/drm/nouveau/nouveau_gem.c
> index 5b27845075a1..d476940ee97c 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_gem.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_gem.c
> @@ -339,7 +339,7 @@ nouveau_gem_set_domain(struct drm_gem_object *gem, uint32_t read_domains,
> struct ttm_buffer_object *bo = &nvbo->bo;
> uint32_t domains = valid_domains & nvbo->valid_domains &
> (write_domains ? write_domains : read_domains);
> - uint32_t pref_domains = 0;;
> + uint32_t pref_domains = 0;
>
> if (!domains)
> return -EINVAL;
> diff --git a/drivers/net/ethernet/mediatek/mtk_star_emac.c b/drivers/net/ethernet/mediatek/mtk_star_emac.c
> index 96d2891f1675..0a445dde67ae 100644
> --- a/drivers/net/ethernet/mediatek/mtk_star_emac.c
> +++ b/drivers/net/ethernet/mediatek/mtk_star_emac.c
> @@ -1086,7 +1086,7 @@ static void mtk_star_tx_complete_all(struct mtk_star_priv *priv)
>
> spin_lock(&priv->lock);
>
> - for (pkts_compl = 0, bytes_compl = 0;;
> + for (pkts_compl = 0, bytes_compl = 0;
> pkts_compl++, bytes_compl += ret, wake = true) {
Was this driver buggy?
Do you intend to modify the behavior of this driver?
Are you sure that your change does not modify the behaviour of this
driver? Is it a purely syntactic change here? Does it even compile
after your change?
> if (!mtk_star_ring_descs_available(ring))
> break;
> diff --git a/fs/block_dev.c b/fs/block_dev.c
> index 9ef4f1fc2cb0..61b938fa5d5b 100644
> --- a/fs/block_dev.c
> +++ b/fs/block_dev.c
> @@ -1268,7 +1268,7 @@ static int blkdev_get_whole(struct block_device *bdev, fmode_t mode)
> if (test_bit(GD_NEED_PART_SCAN, &disk->state))
> bdev_disk_changed(disk, false);
> bdev->bd_openers++;
> - return 0;;
> + return 0;
> }
>
> static void blkdev_put_whole(struct block_device *bdev, fmode_t mode)
> diff --git a/include/trace/events/iocost.h b/include/trace/events/iocost.h
> index e282ce02fa2d..6d1626e7a4ce 100644
> --- a/include/trace/events/iocost.h
> +++ b/include/trace/events/iocost.h
> @@ -160,7 +160,7 @@ TRACE_EVENT(iocost_ioc_vrate_adj,
>
> TP_fast_assign(
> __assign_str(devname, ioc_name(ioc));
> - __entry->old_vrate = atomic64_read(&ioc->vtime_rate);;
> + __entry->old_vrate = atomic64_read(&ioc->vtime_rate);
> __entry->new_vrate = new_vrate;
> __entry->busy_level = ioc->busy_level;
> __entry->read_missed_ppm = missed_ppm[READ];
> diff --git a/net/sched/cls_rsvp.h b/net/sched/cls_rsvp.h
> index 27a4b6dbcf57..b7929dfcca1f 100644
> --- a/net/sched/cls_rsvp.h
> +++ b/net/sched/cls_rsvp.h
> @@ -228,7 +228,7 @@ static void rsvp_replace(struct tcf_proto *tp, struct rsvp_filter *n, u32 h)
>
> for (s = rtnl_dereference(head->ht[h1]); s;
> s = rtnl_dereference(s->next)) {
> - for (ins = &s->ht[h2], pins = rtnl_dereference(*ins); ;
> + for (ins = &s->ht[h2], pins = rtnl_dereference(*ins);
Again:
Was this code buggy?
Do you intend to modify the behavior of this code?
Are you sure that your change does not modify the behaviour of this
code? Is it a purely syntactic change here? Does it even compile after
your change?
> ins = &pins->next, pins = rtnl_dereference(*ins)) {
> if (pins->handle == h) {
> RCU_INIT_POINTER(n->next, pins->next);
> diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
> index 6e1abe805448..290359f6ce6f 100644
> --- a/net/sched/cls_u32.c
> +++ b/net/sched/cls_u32.c
> @@ -779,7 +779,7 @@ static void u32_replace_knode(struct tcf_proto *tp, struct tc_u_common *tp_c,
> /* The node must always exist for it to be replaced if this is not the
> * case then something went very wrong elsewhere.
> */
> - for (pins = rtnl_dereference(*ins); ;
> + for (pins = rtnl_dereference(*ins);
> ins = &pins->next, pins = rtnl_dereference(*ins))
Again:
Was this code buggy?
Do you intend to modify the behavior of this code?
Are you sure that your change does not modify the behaviour of this
code? Is it a purely syntactic change here? Does it even compile after
your change?
> if (pins->handle == n->handle)
> break;
> --
> 2.25.1
>
Overall: please think more critically about what code you are
modifying and why. Just that checkpatch mentions some pattern, does
not mean that the change is always reasonable (Think for yourself!).
Maybe, you can think about a change to the rule so that a certain
pattern is not indicated anymore... and after some discussion, show
that you can IMPLEMENT such change to checkpatch. You want to
contribute improvements to checkpatch and WE WANT you to contribute as
well.
Lukas
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-08-02 6:55 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-01 23:08 [PATCH 2/2] Task 2: checkpath.pl documentation mentorship: remove semicolon warnings Rahul Balaji
2021-08-02 6:57 ` Lukas Bulwahn
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.