* [PATCH] md: fix a build warning
@ 2015-06-10 15:20 ` Firo Yang
0 siblings, 0 replies; 16+ messages in thread
From: Firo Yang @ 2015-06-10 15:20 UTC (permalink / raw)
To: neilb; +Cc: linux-raid, kernel-janitors, Firo Yang
Warning like this:
drivers/md/md.c: In function ‘update_array_info’:
drivers/md/md.c:6394:26: warning: logical not is only applied
to the left hand side of comparison [-Wlogical-not-parentheses]
!mddev->persistent != info->not_persistent||
I fix it by enclosing !mddev->persistent with parentheses
By the way, I also fixed a line over 80 characters warning outputed
by ./scripts/checkpatch.pl
Signed-off-by: Firo Yang <firogm@gmail.com>
---
drivers/md/md.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index dd85be9..b420d82 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -6391,9 +6391,10 @@ static int update_array_info(struct mddev *mddev, mdu_array_info_t *info)
mddev->ctime != info->ctime ||
mddev->level != info->level ||
/* mddev->layout != info->layout || */
- !mddev->persistent != info->not_persistent||
+ (!mddev->persistent) != info->not_persistent ||
mddev->chunk_sectors != info->chunk_size >> 9 ||
- /* ignore bottom 8 bits of state, and allow SB_BITMAP_PRESENT to change */
+ /* ignore bottom 8 bits of state, and allow SB_BITMAP_PRESENT
+ to change */
((state^info->state) & 0xfffffe00)
)
return -EINVAL;
--
2.4.2
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH] md: fix a build warning @ 2015-06-10 15:20 ` Firo Yang 0 siblings, 0 replies; 16+ messages in thread From: Firo Yang @ 2015-06-10 15:20 UTC (permalink / raw) To: neilb; +Cc: linux-raid, kernel-janitors, Firo Yang Warning like this: drivers/md/md.c: In function ‘update_array_info’: drivers/md/md.c:6394:26: warning: logical not is only applied to the left hand side of comparison [-Wlogical-not-parentheses] !mddev->persistent != info->not_persistent|| I fix it by enclosing !mddev->persistent with parentheses By the way, I also fixed a line over 80 characters warning outputed by ./scripts/checkpatch.pl Signed-off-by: Firo Yang <firogm@gmail.com> --- drivers/md/md.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/md/md.c b/drivers/md/md.c index dd85be9..b420d82 100644 --- a/drivers/md/md.c +++ b/drivers/md/md.c @@ -6391,9 +6391,10 @@ static int update_array_info(struct mddev *mddev, mdu_array_info_t *info) mddev->ctime != info->ctime || mddev->level != info->level || /* mddev->layout != info->layout || */ - !mddev->persistent != info->not_persistent|| + (!mddev->persistent) != info->not_persistent || mddev->chunk_sectors != info->chunk_size >> 9 || - /* ignore bottom 8 bits of state, and allow SB_BITMAP_PRESENT to change */ + /* ignore bottom 8 bits of state, and allow SB_BITMAP_PRESENT + to change */ ((state^info->state) & 0xfffffe00) ) return -EINVAL; -- 2.4.2 -- To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH] md: fix a build warning 2015-06-10 15:20 ` Firo Yang @ 2015-06-10 16:07 ` Dan Carpenter -1 siblings, 0 replies; 16+ messages in thread From: Dan Carpenter @ 2015-06-10 16:07 UTC (permalink / raw) To: Firo Yang; +Cc: neilb, linux-raid, kernel-janitors On Wed, Jun 10, 2015 at 11:20:58PM +0800, Firo Yang wrote: > By the way, I also fixed a line over 80 characters warning outputed > by ./scripts/checkpatch.pl Don't do this. It's not on the same line, it's not really related at all. > mddev->chunk_sectors != info->chunk_size >> 9 || > - /* ignore bottom 8 bits of state, and allow SB_BITMAP_PRESENT to change */ > + /* ignore bottom 8 bits of state, and allow SB_BITMAP_PRESENT > + to change */ The new comment style isn't correct. regards, dan carpenter ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] md: fix a build warning @ 2015-06-10 16:07 ` Dan Carpenter 0 siblings, 0 replies; 16+ messages in thread From: Dan Carpenter @ 2015-06-10 16:07 UTC (permalink / raw) To: Firo Yang; +Cc: neilb, linux-raid, kernel-janitors On Wed, Jun 10, 2015 at 11:20:58PM +0800, Firo Yang wrote: > By the way, I also fixed a line over 80 characters warning outputed > by ./scripts/checkpatch.pl Don't do this. It's not on the same line, it's not really related at all. > mddev->chunk_sectors != info->chunk_size >> 9 || > - /* ignore bottom 8 bits of state, and allow SB_BITMAP_PRESENT to change */ > + /* ignore bottom 8 bits of state, and allow SB_BITMAP_PRESENT > + to change */ The new comment style isn't correct. regards, dan carpenter ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] md: fix a build warning 2015-06-10 15:20 ` Firo Yang @ 2015-06-10 17:32 ` walter harms -1 siblings, 0 replies; 16+ messages in thread From: walter harms @ 2015-06-10 17:32 UTC (permalink / raw) To: Firo Yang; +Cc: neilb, linux-raid, kernel-janitors Am 10.06.2015 17:20, schrieb Firo Yang: > Warning like this: > > drivers/md/md.c: In function ‘update_array_info’: > drivers/md/md.c:6394:26: warning: logical not is only applied > to the left hand side of comparison [-Wlogical-not-parentheses] > !mddev->persistent != info->not_persistent|| > > I fix it by enclosing !mddev->persistent with parentheses > > By the way, I also fixed a line over 80 characters warning outputed > by ./scripts/checkpatch.pl > > Signed-off-by: Firo Yang <firogm@gmail.com> > --- > drivers/md/md.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/md/md.c b/drivers/md/md.c > index dd85be9..b420d82 100644 > --- a/drivers/md/md.c > +++ b/drivers/md/md.c > @@ -6391,9 +6391,10 @@ static int update_array_info(struct mddev *mddev, mdu_array_info_t *info) > mddev->ctime != info->ctime || > mddev->level != info->level || > /* mddev->layout != info->layout || */ > - !mddev->persistent != info->not_persistent|| > + (!mddev->persistent) != info->not_persistent || this looks odd, would it be possible the check for = instead (and drop the !) ? and it someone care for readability: It would be helpful to make some more ifs here. re, wh > mddev->chunk_sectors != info->chunk_size >> 9 || > - /* ignore bottom 8 bits of state, and allow SB_BITMAP_PRESENT to change */ > + /* ignore bottom 8 bits of state, and allow SB_BITMAP_PRESENT > + to change */ > ((state^info->state) & 0xfffffe00) > ) > return -EINVAL; ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] md: fix a build warning @ 2015-06-10 17:32 ` walter harms 0 siblings, 0 replies; 16+ messages in thread From: walter harms @ 2015-06-10 17:32 UTC (permalink / raw) To: Firo Yang; +Cc: neilb, linux-raid, kernel-janitors Am 10.06.2015 17:20, schrieb Firo Yang: > Warning like this: > > drivers/md/md.c: In function ‘update_array_info’: > drivers/md/md.c:6394:26: warning: logical not is only applied > to the left hand side of comparison [-Wlogical-not-parentheses] > !mddev->persistent != info->not_persistent|| > > I fix it by enclosing !mddev->persistent with parentheses > > By the way, I also fixed a line over 80 characters warning outputed > by ./scripts/checkpatch.pl > > Signed-off-by: Firo Yang <firogm@gmail.com> > --- > drivers/md/md.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/md/md.c b/drivers/md/md.c > index dd85be9..b420d82 100644 > --- a/drivers/md/md.c > +++ b/drivers/md/md.c > @@ -6391,9 +6391,10 @@ static int update_array_info(struct mddev *mddev, mdu_array_info_t *info) > mddev->ctime != info->ctime || > mddev->level != info->level || > /* mddev->layout != info->layout || */ > - !mddev->persistent != info->not_persistent|| > + (!mddev->persistent) != info->not_persistent || this looks odd, would it be possible the check for == instead (and drop the !) ? and it someone care for readability: It would be helpful to make some more ifs here. re, wh > mddev->chunk_sectors != info->chunk_size >> 9 || > - /* ignore bottom 8 bits of state, and allow SB_BITMAP_PRESENT to change */ > + /* ignore bottom 8 bits of state, and allow SB_BITMAP_PRESENT > + to change */ > ((state^info->state) & 0xfffffe00) > ) > return -EINVAL; -- To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] md: fix a build warning 2015-06-10 17:32 ` walter harms @ 2015-06-10 17:36 ` Julia Lawall -1 siblings, 0 replies; 16+ messages in thread From: Julia Lawall @ 2015-06-10 17:36 UTC (permalink / raw) To: walter harms; +Cc: Firo Yang, neilb, linux-raid, kernel-janitors [-- Attachment #1: Type: TEXT/PLAIN, Size: 2098 bytes --] On Wed, 10 Jun 2015, walter harms wrote: > > > Am 10.06.2015 17:20, schrieb Firo Yang: > > Warning like this: > > > > drivers/md/md.c: In function ‘update_array_info’: > > drivers/md/md.c:6394:26: warning: logical not is only applied > > to the left hand side of comparison [-Wlogical-not-parentheses] > > !mddev->persistent != info->not_persistent|| > > > > I fix it by enclosing !mddev->persistent with parentheses > > > > By the way, I also fixed a line over 80 characters warning outputed > > by ./scripts/checkpatch.pl > > > > Signed-off-by: Firo Yang <firogm@gmail.com> > > --- > > drivers/md/md.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/md/md.c b/drivers/md/md.c > > index dd85be9..b420d82 100644 > > --- a/drivers/md/md.c > > +++ b/drivers/md/md.c > > @@ -6391,9 +6391,10 @@ static int update_array_info(struct mddev *mddev, mdu_array_info_t *info) > > mddev->ctime != info->ctime || > > mddev->level != info->level || > > /* mddev->layout != info->layout || */ > > - !mddev->persistent != info->not_persistent|| > > + (!mddev->persistent) != info->not_persistent || > > > this looks odd, > would it be possible the check for == instead (and drop the !) ? > and it someone care for readability: It would be helpful to > make some more ifs here. The original issue looks like a false positive. If all of the other cases have no parentheses on the left and use !=, isn't it better to leave the code as is? julia > > re, > wh > > > mddev->chunk_sectors != info->chunk_size >> 9 || > > - /* ignore bottom 8 bits of state, and allow SB_BITMAP_PRESENT to change */ > > + /* ignore bottom 8 bits of state, and allow SB_BITMAP_PRESENT > > + to change */ > > ((state^info->state) & 0xfffffe00) > > ) > > return -EINVAL; > -- > To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] md: fix a build warning @ 2015-06-10 17:36 ` Julia Lawall 0 siblings, 0 replies; 16+ messages in thread From: Julia Lawall @ 2015-06-10 17:36 UTC (permalink / raw) To: walter harms; +Cc: Firo Yang, neilb, linux-raid, kernel-janitors [-- Attachment #1: Type: TEXT/PLAIN, Size: 2098 bytes --] On Wed, 10 Jun 2015, walter harms wrote: > > > Am 10.06.2015 17:20, schrieb Firo Yang: > > Warning like this: > > > > drivers/md/md.c: In function ‘update_array_info’: > > drivers/md/md.c:6394:26: warning: logical not is only applied > > to the left hand side of comparison [-Wlogical-not-parentheses] > > !mddev->persistent != info->not_persistent|| > > > > I fix it by enclosing !mddev->persistent with parentheses > > > > By the way, I also fixed a line over 80 characters warning outputed > > by ./scripts/checkpatch.pl > > > > Signed-off-by: Firo Yang <firogm@gmail.com> > > --- > > drivers/md/md.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/md/md.c b/drivers/md/md.c > > index dd85be9..b420d82 100644 > > --- a/drivers/md/md.c > > +++ b/drivers/md/md.c > > @@ -6391,9 +6391,10 @@ static int update_array_info(struct mddev *mddev, mdu_array_info_t *info) > > mddev->ctime != info->ctime || > > mddev->level != info->level || > > /* mddev->layout != info->layout || */ > > - !mddev->persistent != info->not_persistent|| > > + (!mddev->persistent) != info->not_persistent || > > > this looks odd, > would it be possible the check for == instead (and drop the !) ? > and it someone care for readability: It would be helpful to > make some more ifs here. The original issue looks like a false positive. If all of the other cases have no parentheses on the left and use !=, isn't it better to leave the code as is? julia > > re, > wh > > > mddev->chunk_sectors != info->chunk_size >> 9 || > > - /* ignore bottom 8 bits of state, and allow SB_BITMAP_PRESENT to change */ > > + /* ignore bottom 8 bits of state, and allow SB_BITMAP_PRESENT > > + to change */ > > ((state^info->state) & 0xfffffe00) > > ) > > return -EINVAL; > -- > To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] md: fix a build warning 2015-06-10 17:36 ` Julia Lawall @ 2015-06-10 18:04 ` walter harms -1 siblings, 0 replies; 16+ messages in thread From: walter harms @ 2015-06-10 18:04 UTC (permalink / raw) To: Julia Lawall; +Cc: Firo Yang, neilb, linux-raid, kernel-janitors Am 10.06.2015 19:36, schrieb Julia Lawall: > > > On Wed, 10 Jun 2015, walter harms wrote: > >> >> >> Am 10.06.2015 17:20, schrieb Firo Yang: >>> Warning like this: >>> >>> drivers/md/md.c: In function ‘update_array_info’: >>> drivers/md/md.c:6394:26: warning: logical not is only applied >>> to the left hand side of comparison [-Wlogical-not-parentheses] >>> !mddev->persistent != info->not_persistent|| >>> >>> I fix it by enclosing !mddev->persistent with parentheses >>> >>> By the way, I also fixed a line over 80 characters warning outputed >>> by ./scripts/checkpatch.pl >>> >>> Signed-off-by: Firo Yang <firogm@gmail.com> >>> --- >>> drivers/md/md.c | 5 +++-- >>> 1 file changed, 3 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/md/md.c b/drivers/md/md.c >>> index dd85be9..b420d82 100644 >>> --- a/drivers/md/md.c >>> +++ b/drivers/md/md.c >>> @@ -6391,9 +6391,10 @@ static int update_array_info(struct mddev *mddev, mdu_array_info_t *info) >>> mddev->ctime != info->ctime || >>> mddev->level != info->level || >>> /* mddev->layout != info->layout || */ >>> - !mddev->persistent != info->not_persistent|| >>> + (!mddev->persistent) != info->not_persistent || >> >> >> this looks odd, >> would it be possible the check for = instead (and drop the !) ? >> and it someone care for readability: It would be helpful to >> make some more ifs here. > > The original issue looks like a false positive. If all of the other cases > have no parentheses on the left and use !=, isn't it better to leave the > code as is? > i do not think so, the name indicate persistent/not_persistent devices. Maybe both parties can agree what should be stored, ppl are bad at those not_not things and that is an unnecessary trouble spot. re, wh > julia > >> >> re, >> wh >> >>> mddev->chunk_sectors != info->chunk_size >> 9 || >>> - /* ignore bottom 8 bits of state, and allow SB_BITMAP_PRESENT to change */ >>> + /* ignore bottom 8 bits of state, and allow SB_BITMAP_PRESENT >>> + to change */ >>> ((state^info->state) & 0xfffffe00) >>> ) >>> return -EINVAL; >> -- >> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] md: fix a build warning @ 2015-06-10 18:04 ` walter harms 0 siblings, 0 replies; 16+ messages in thread From: walter harms @ 2015-06-10 18:04 UTC (permalink / raw) To: Julia Lawall; +Cc: Firo Yang, neilb, linux-raid, kernel-janitors Am 10.06.2015 19:36, schrieb Julia Lawall: > > > On Wed, 10 Jun 2015, walter harms wrote: > >> >> >> Am 10.06.2015 17:20, schrieb Firo Yang: >>> Warning like this: >>> >>> drivers/md/md.c: In function ‘update_array_info’: >>> drivers/md/md.c:6394:26: warning: logical not is only applied >>> to the left hand side of comparison [-Wlogical-not-parentheses] >>> !mddev->persistent != info->not_persistent|| >>> >>> I fix it by enclosing !mddev->persistent with parentheses >>> >>> By the way, I also fixed a line over 80 characters warning outputed >>> by ./scripts/checkpatch.pl >>> >>> Signed-off-by: Firo Yang <firogm@gmail.com> >>> --- >>> drivers/md/md.c | 5 +++-- >>> 1 file changed, 3 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/md/md.c b/drivers/md/md.c >>> index dd85be9..b420d82 100644 >>> --- a/drivers/md/md.c >>> +++ b/drivers/md/md.c >>> @@ -6391,9 +6391,10 @@ static int update_array_info(struct mddev *mddev, mdu_array_info_t *info) >>> mddev->ctime != info->ctime || >>> mddev->level != info->level || >>> /* mddev->layout != info->layout || */ >>> - !mddev->persistent != info->not_persistent|| >>> + (!mddev->persistent) != info->not_persistent || >> >> >> this looks odd, >> would it be possible the check for == instead (and drop the !) ? >> and it someone care for readability: It would be helpful to >> make some more ifs here. > > The original issue looks like a false positive. If all of the other cases > have no parentheses on the left and use !=, isn't it better to leave the > code as is? > i do not think so, the name indicate persistent/not_persistent devices. Maybe both parties can agree what should be stored, ppl are bad at those not_not things and that is an unnecessary trouble spot. re, wh > julia > >> >> re, >> wh >> >>> mddev->chunk_sectors != info->chunk_size >> 9 || >>> - /* ignore bottom 8 bits of state, and allow SB_BITMAP_PRESENT to change */ >>> + /* ignore bottom 8 bits of state, and allow SB_BITMAP_PRESENT >>> + to change */ >>> ((state^info->state) & 0xfffffe00) >>> ) >>> return -EINVAL; >> -- >> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> -- To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] md: fix a build warning 2015-06-10 17:36 ` Julia Lawall @ 2015-06-10 20:38 ` Neil Brown -1 siblings, 0 replies; 16+ messages in thread From: Neil Brown @ 2015-06-10 20:38 UTC (permalink / raw) To: Julia Lawall; +Cc: walter harms, Firo Yang, linux-raid, kernel-janitors On Wed, 10 Jun 2015 19:36:43 +0200 (CEST) Julia Lawall <julia.lawall@lip6.fr> wrote: > > > On Wed, 10 Jun 2015, walter harms wrote: > > > > > > > Am 10.06.2015 17:20, schrieb Firo Yang: > > > Warning like this: > > > > > > drivers/md/md.c: In function ‘update_array_info’: > > > drivers/md/md.c:6394:26: warning: logical not is only applied > > > to the left hand side of comparison [-Wlogical-not-parentheses] > > > !mddev->persistent != info->not_persistent|| > > > > > > I fix it by enclosing !mddev->persistent with parentheses > > > > > > By the way, I also fixed a line over 80 characters warning outputed > > > by ./scripts/checkpatch.pl > > > > > > Signed-off-by: Firo Yang <firogm@gmail.com> > > > --- > > > drivers/md/md.c | 5 +++-- > > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/md/md.c b/drivers/md/md.c > > > index dd85be9..b420d82 100644 > > > --- a/drivers/md/md.c > > > +++ b/drivers/md/md.c > > > @@ -6391,9 +6391,10 @@ static int update_array_info(struct mddev *mddev, mdu_array_info_t *info) > > > mddev->ctime != info->ctime || > > > mddev->level != info->level || > > > /* mddev->layout != info->layout || */ > > > - !mddev->persistent != info->not_persistent|| > > > + (!mddev->persistent) != info->not_persistent || > > > > > > this looks odd, > > would it be possible the check for = instead (and drop the !) ? > > and it someone care for readability: It would be helpful to > > make some more ifs here. > > The original issue looks like a false positive. If all of the other cases > have no parentheses on the left and use !=, isn't it better to leave the > code as is? > Leaving the code as it is would leave at least one compiler generating warnings. I don't like warnings. So if the compiler can be "fixed" that would be good. But it seems unlikely. May we could do: > > > + mddev->persistent != !info->not_persistent || That would keep the two "not"s together, avoid the need for parentheses, and probably avoid the compiler warning. (I think I had it the way it is because it almost reads " not ...peristent != ... not_persistent" but that isn't a strong reason) Firo: Can you confirm that this version doesn't upset the compiler? If if doesn't I'd definitely prefer this one. (and definitely leave out the comment change - just change the code) Thanks, NeilBrown > julia > > > > > re, > > wh > > > > > mddev->chunk_sectors != info->chunk_size >> 9 || > > > - /* ignore bottom 8 bits of state, and allow SB_BITMAP_PRESENT to change */ > > > + /* ignore bottom 8 bits of state, and allow SB_BITMAP_PRESENT > > > + to change */ > > > ((state^info->state) & 0xfffffe00) > > > ) > > > return -EINVAL; > > -- > > To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] md: fix a build warning @ 2015-06-10 20:38 ` Neil Brown 0 siblings, 0 replies; 16+ messages in thread From: Neil Brown @ 2015-06-10 20:38 UTC (permalink / raw) To: Julia Lawall; +Cc: walter harms, Firo Yang, linux-raid, kernel-janitors On Wed, 10 Jun 2015 19:36:43 +0200 (CEST) Julia Lawall <julia.lawall@lip6.fr> wrote: > > > On Wed, 10 Jun 2015, walter harms wrote: > > > > > > > Am 10.06.2015 17:20, schrieb Firo Yang: > > > Warning like this: > > > > > > drivers/md/md.c: In function ‘update_array_info’: > > > drivers/md/md.c:6394:26: warning: logical not is only applied > > > to the left hand side of comparison [-Wlogical-not-parentheses] > > > !mddev->persistent != info->not_persistent|| > > > > > > I fix it by enclosing !mddev->persistent with parentheses > > > > > > By the way, I also fixed a line over 80 characters warning outputed > > > by ./scripts/checkpatch.pl > > > > > > Signed-off-by: Firo Yang <firogm@gmail.com> > > > --- > > > drivers/md/md.c | 5 +++-- > > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/md/md.c b/drivers/md/md.c > > > index dd85be9..b420d82 100644 > > > --- a/drivers/md/md.c > > > +++ b/drivers/md/md.c > > > @@ -6391,9 +6391,10 @@ static int update_array_info(struct mddev *mddev, mdu_array_info_t *info) > > > mddev->ctime != info->ctime || > > > mddev->level != info->level || > > > /* mddev->layout != info->layout || */ > > > - !mddev->persistent != info->not_persistent|| > > > + (!mddev->persistent) != info->not_persistent || > > > > > > this looks odd, > > would it be possible the check for == instead (and drop the !) ? > > and it someone care for readability: It would be helpful to > > make some more ifs here. > > The original issue looks like a false positive. If all of the other cases > have no parentheses on the left and use !=, isn't it better to leave the > code as is? > Leaving the code as it is would leave at least one compiler generating warnings. I don't like warnings. So if the compiler can be "fixed" that would be good. But it seems unlikely. May we could do: > > > + mddev->persistent != !info->not_persistent || That would keep the two "not"s together, avoid the need for parentheses, and probably avoid the compiler warning. (I think I had it the way it is because it almost reads " not ...peristent != ... not_persistent" but that isn't a strong reason) Firo: Can you confirm that this version doesn't upset the compiler? If if doesn't I'd definitely prefer this one. (and definitely leave out the comment change - just change the code) Thanks, NeilBrown > julia > > > > > re, > > wh > > > > > mddev->chunk_sectors != info->chunk_size >> 9 || > > > - /* ignore bottom 8 bits of state, and allow SB_BITMAP_PRESENT to change */ > > > + /* ignore bottom 8 bits of state, and allow SB_BITMAP_PRESENT > > > + to change */ > > > ((state^info->state) & 0xfffffe00) > > > ) > > > return -EINVAL; > > -- > > To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > -- To unsubscribe from this list: send the line "unsubscribe linux-raid" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] md: fix a build warning 2015-06-10 17:32 ` walter harms @ 2015-06-10 20:34 ` Neil Brown -1 siblings, 0 replies; 16+ messages in thread From: Neil Brown @ 2015-06-10 20:34 UTC (permalink / raw) To: walter harms; +Cc: Firo Yang, linux-raid, kernel-janitors On Wed, 10 Jun 2015 19:32:41 +0200 walter harms <wharms@bfs.de> wrote: > > > Am 10.06.2015 17:20, schrieb Firo Yang: > > Warning like this: > > > > drivers/md/md.c: In function ‘update_array_info’: > > drivers/md/md.c:6394:26: warning: logical not is only applied > > to the left hand side of comparison [-Wlogical-not-parentheses] > > !mddev->persistent != info->not_persistent|| > > > > I fix it by enclosing !mddev->persistent with parentheses > > > > By the way, I also fixed a line over 80 characters warning outputed > > by ./scripts/checkpatch.pl > > > > Signed-off-by: Firo Yang <firogm@gmail.com> > > --- > > drivers/md/md.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/md/md.c b/drivers/md/md.c > > index dd85be9..b420d82 100644 > > --- a/drivers/md/md.c > > +++ b/drivers/md/md.c > > @@ -6391,9 +6391,10 @@ static int update_array_info(struct mddev *mddev, mdu_array_info_t *info) > > mddev->ctime != info->ctime || > > mddev->level != info->level || > > /* mddev->layout != info->layout || */ > > - !mddev->persistent != info->not_persistent|| > > + (!mddev->persistent) != info->not_persistent || > > > this looks odd, > would it be possible the check for = instead (and drop the !) ? > and it someone care for readability: It would be helpful to > make some more ifs here. The first attempt Firo summited did exactly that. I said no. I like the visual consistency of LHS != RHS. More 'if's would just waste more vertical space. Thanks, NeilBrown > > re, > wh > > > mddev->chunk_sectors != info->chunk_size >> 9 || > > - /* ignore bottom 8 bits of state, and allow SB_BITMAP_PRESENT to change */ > > + /* ignore bottom 8 bits of state, and allow SB_BITMAP_PRESENT > > + to change */ > > ((state^info->state) & 0xfffffe00) > > ) > > return -EINVAL; ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] md: fix a build warning @ 2015-06-10 20:34 ` Neil Brown 0 siblings, 0 replies; 16+ messages in thread From: Neil Brown @ 2015-06-10 20:34 UTC (permalink / raw) To: walter harms; +Cc: Firo Yang, linux-raid, kernel-janitors On Wed, 10 Jun 2015 19:32:41 +0200 walter harms <wharms@bfs.de> wrote: > > > Am 10.06.2015 17:20, schrieb Firo Yang: > > Warning like this: > > > > drivers/md/md.c: In function ‘update_array_info’: > > drivers/md/md.c:6394:26: warning: logical not is only applied > > to the left hand side of comparison [-Wlogical-not-parentheses] > > !mddev->persistent != info->not_persistent|| > > > > I fix it by enclosing !mddev->persistent with parentheses > > > > By the way, I also fixed a line over 80 characters warning outputed > > by ./scripts/checkpatch.pl > > > > Signed-off-by: Firo Yang <firogm@gmail.com> > > --- > > drivers/md/md.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/md/md.c b/drivers/md/md.c > > index dd85be9..b420d82 100644 > > --- a/drivers/md/md.c > > +++ b/drivers/md/md.c > > @@ -6391,9 +6391,10 @@ static int update_array_info(struct mddev *mddev, mdu_array_info_t *info) > > mddev->ctime != info->ctime || > > mddev->level != info->level || > > /* mddev->layout != info->layout || */ > > - !mddev->persistent != info->not_persistent|| > > + (!mddev->persistent) != info->not_persistent || > > > this looks odd, > would it be possible the check for == instead (and drop the !) ? > and it someone care for readability: It would be helpful to > make some more ifs here. The first attempt Firo summited did exactly that. I said no. I like the visual consistency of LHS != RHS. More 'if's would just waste more vertical space. Thanks, NeilBrown > > re, > wh > > > mddev->chunk_sectors != info->chunk_size >> 9 || > > - /* ignore bottom 8 bits of state, and allow SB_BITMAP_PRESENT to change */ > > + /* ignore bottom 8 bits of state, and allow SB_BITMAP_PRESENT > > + to change */ > > ((state^info->state) & 0xfffffe00) > > ) > > return -EINVAL; -- To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] md: fix a build warning 2015-06-10 15:20 ` Firo Yang @ 2015-06-11 11:48 ` Dan Carpenter -1 siblings, 0 replies; 16+ messages in thread From: Dan Carpenter @ 2015-06-11 11:48 UTC (permalink / raw) To: Firo Yang, Joe Perches; +Cc: neilb, linux-raid, kernel-janitors Hey Joe, checkpatch.pl in linux-next is now encouraging people to make unrelated long line fixes like the below patch. Reverting 0b4193c79c83 ('checkpatch: categorize some long line length checks') makes the new checkpatch.pl warning go away. regards, dan carpenter On Wed, Jun 10, 2015 at 11:20:58PM +0800, Firo Yang wrote: > Warning like this: > > drivers/md/md.c: In function ‘update_array_info’: > drivers/md/md.c:6394:26: warning: logical not is only applied > to the left hand side of comparison [-Wlogical-not-parentheses] > !mddev->persistent != info->not_persistent|| > > I fix it by enclosing !mddev->persistent with parentheses > > By the way, I also fixed a line over 80 characters warning outputed > by ./scripts/checkpatch.pl > > Signed-off-by: Firo Yang <firogm@gmail.com> > --- > drivers/md/md.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/md/md.c b/drivers/md/md.c > index dd85be9..b420d82 100644 > --- a/drivers/md/md.c > +++ b/drivers/md/md.c > @@ -6391,9 +6391,10 @@ static int update_array_info(struct mddev *mddev, mdu_array_info_t *info) > mddev->ctime != info->ctime || > mddev->level != info->level || > /* mddev->layout != info->layout || */ > - !mddev->persistent != info->not_persistent|| > + (!mddev->persistent) != info->not_persistent || > mddev->chunk_sectors != info->chunk_size >> 9 || > - /* ignore bottom 8 bits of state, and allow SB_BITMAP_PRESENT to change */ > + /* ignore bottom 8 bits of state, and allow SB_BITMAP_PRESENT > + to change */ > ((state^info->state) & 0xfffffe00) > ) > return -EINVAL; > -- > 2.4.2 > > -- > To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] md: fix a build warning @ 2015-06-11 11:48 ` Dan Carpenter 0 siblings, 0 replies; 16+ messages in thread From: Dan Carpenter @ 2015-06-11 11:48 UTC (permalink / raw) To: Firo Yang, Joe Perches; +Cc: neilb, linux-raid, kernel-janitors Hey Joe, checkpatch.pl in linux-next is now encouraging people to make unrelated long line fixes like the below patch. Reverting 0b4193c79c83 ('checkpatch: categorize some long line length checks') makes the new checkpatch.pl warning go away. regards, dan carpenter On Wed, Jun 10, 2015 at 11:20:58PM +0800, Firo Yang wrote: > Warning like this: > > drivers/md/md.c: In function ‘update_array_info’: > drivers/md/md.c:6394:26: warning: logical not is only applied > to the left hand side of comparison [-Wlogical-not-parentheses] > !mddev->persistent != info->not_persistent|| > > I fix it by enclosing !mddev->persistent with parentheses > > By the way, I also fixed a line over 80 characters warning outputed > by ./scripts/checkpatch.pl > > Signed-off-by: Firo Yang <firogm@gmail.com> > --- > drivers/md/md.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/md/md.c b/drivers/md/md.c > index dd85be9..b420d82 100644 > --- a/drivers/md/md.c > +++ b/drivers/md/md.c > @@ -6391,9 +6391,10 @@ static int update_array_info(struct mddev *mddev, mdu_array_info_t *info) > mddev->ctime != info->ctime || > mddev->level != info->level || > /* mddev->layout != info->layout || */ > - !mddev->persistent != info->not_persistent|| > + (!mddev->persistent) != info->not_persistent || > mddev->chunk_sectors != info->chunk_size >> 9 || > - /* ignore bottom 8 bits of state, and allow SB_BITMAP_PRESENT to change */ > + /* ignore bottom 8 bits of state, and allow SB_BITMAP_PRESENT > + to change */ > ((state^info->state) & 0xfffffe00) > ) > return -EINVAL; > -- > 2.4.2 > > -- > To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2015-06-11 11:48 UTC | newest] Thread overview: 16+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-06-10 15:20 [PATCH] md: fix a build warning Firo Yang 2015-06-10 15:20 ` Firo Yang 2015-06-10 16:07 ` Dan Carpenter 2015-06-10 16:07 ` Dan Carpenter 2015-06-10 17:32 ` walter harms 2015-06-10 17:32 ` walter harms 2015-06-10 17:36 ` Julia Lawall 2015-06-10 17:36 ` Julia Lawall 2015-06-10 18:04 ` walter harms 2015-06-10 18:04 ` walter harms 2015-06-10 20:38 ` Neil Brown 2015-06-10 20:38 ` Neil Brown 2015-06-10 20:34 ` Neil Brown 2015-06-10 20:34 ` Neil Brown 2015-06-11 11:48 ` Dan Carpenter 2015-06-11 11:48 ` Dan Carpenter
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.