From: Lars Ellenberg <lars.ellenberg@linbit.com>
To: Martin Kepplinger <martink@posteo.de>
Cc: drbd-dev@lists.linbit.com, linux-kernel@vger.kernel.org,
rientjes@google.com
Subject: Re: [Drbd-dev] [PATCH v2] drbd: change one-bit bitfield to be an unsigned int
Date: Tue, 17 Jun 2014 12:53:48 +0200 [thread overview]
Message-ID: <20140617105348.GH28884@soda.linbit> (raw)
In-Reply-To: <1402988047-16364-1-git-send-email-martink@posteo.de>
On Tue, Jun 17, 2014 at 08:54:07AM +0200, Martin Kepplinger wrote:
> The one-bit bitfields are assigned true (1) or false (0) and checked
> for them respectively. While it should work either way and -1 is true
> as well it is more clear to see what's going on when using an unsigned int
> because 1 doesn't silently become -1 behind the label true.
>
> Signed-off-by: Martin Kepplinger <martink@posteo.de>
> ---
> Thanks for looking at it. This is more of a question: Does this make sense
> to you now? I can be mistaken. It just wasn't totally clear to me at first
> sight and even though it should be, why not try to improve it.
>
> sparse called it 'dubious' before the change.
>
> (built but untested)
Patch is completely pointless, really,
and I don't get the motivation for it at all.
BUT.
If it makes someone happy,
and since all of our other bitfields in fact are declared "unsigned xyz:N",
for consistency we can make this unsigned as well.
Will probably fold this with some upcoming change in that struct anyways.
Lars
>
> drivers/block/drbd/drbd_interval.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/block/drbd/drbd_interval.h b/drivers/block/drbd/drbd_interval.h
> index f38fcb0..8d670e6 100644
> --- a/drivers/block/drbd/drbd_interval.h
> +++ b/drivers/block/drbd/drbd_interval.h
> @@ -9,8 +9,8 @@ struct drbd_interval {
> sector_t sector; /* start sector of the interval */
> unsigned int size; /* size in bytes */
> sector_t end; /* highest interval end in subtree */
> - int local:1 /* local or remote request? */;
> - int waiting:1;
> + unsigned int local:1; /* local or remote request? */
> + unsigned int waiting:1;
> };
>
> static inline void drbd_clear_interval(struct drbd_interval *i)
WARNING: multiple messages have this Message-ID (diff)
From: Lars Ellenberg <lars.ellenberg@linbit.com>
To: Martin Kepplinger <martink@posteo.de>
Cc: rientjes@google.com, linux-kernel@vger.kernel.org,
drbd-dev@lists.linbit.com
Subject: Re: [Drbd-dev] [PATCH v2] drbd: change one-bit bitfield to be an unsigned int
Date: Tue, 17 Jun 2014 12:53:48 +0200 [thread overview]
Message-ID: <20140617105348.GH28884@soda.linbit> (raw)
In-Reply-To: <1402988047-16364-1-git-send-email-martink@posteo.de>
On Tue, Jun 17, 2014 at 08:54:07AM +0200, Martin Kepplinger wrote:
> The one-bit bitfields are assigned true (1) or false (0) and checked
> for them respectively. While it should work either way and -1 is true
> as well it is more clear to see what's going on when using an unsigned int
> because 1 doesn't silently become -1 behind the label true.
>
> Signed-off-by: Martin Kepplinger <martink@posteo.de>
> ---
> Thanks for looking at it. This is more of a question: Does this make sense
> to you now? I can be mistaken. It just wasn't totally clear to me at first
> sight and even though it should be, why not try to improve it.
>
> sparse called it 'dubious' before the change.
>
> (built but untested)
Patch is completely pointless, really,
and I don't get the motivation for it at all.
BUT.
If it makes someone happy,
and since all of our other bitfields in fact are declared "unsigned xyz:N",
for consistency we can make this unsigned as well.
Will probably fold this with some upcoming change in that struct anyways.
Lars
>
> drivers/block/drbd/drbd_interval.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/block/drbd/drbd_interval.h b/drivers/block/drbd/drbd_interval.h
> index f38fcb0..8d670e6 100644
> --- a/drivers/block/drbd/drbd_interval.h
> +++ b/drivers/block/drbd/drbd_interval.h
> @@ -9,8 +9,8 @@ struct drbd_interval {
> sector_t sector; /* start sector of the interval */
> unsigned int size; /* size in bytes */
> sector_t end; /* highest interval end in subtree */
> - int local:1 /* local or remote request? */;
> - int waiting:1;
> + unsigned int local:1; /* local or remote request? */
> + unsigned int waiting:1;
> };
>
> static inline void drbd_clear_interval(struct drbd_interval *i)
next prev parent reply other threads:[~2014-06-17 10:53 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-06-14 23:07 [Drbd-dev] [PATCH] drbd: change one-bit bitfield to be an unsigned int Martin Kepplinger
2014-06-14 23:07 ` Martin Kepplinger
2014-06-15 0:45 ` [Drbd-dev] [PATCH v2] " Martin Kepplinger
2014-06-15 0:45 ` Martin Kepplinger
2014-06-16 9:28 ` [Drbd-dev] " David Rientjes
2014-06-16 9:28 ` David Rientjes
2014-06-17 6:54 ` [Drbd-dev] " Martin Kepplinger
2014-06-17 6:54 ` Martin Kepplinger
2014-06-17 10:53 ` Lars Ellenberg [this message]
2014-06-17 10:53 ` [Drbd-dev] " Lars Ellenberg
2014-06-17 19:46 ` David Rientjes
2014-06-18 5:50 ` Martin Kepplinger
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20140617105348.GH28884@soda.linbit \
--to=lars.ellenberg@linbit.com \
--cc=drbd-dev@lists.linbit.com \
--cc=linux-kernel@vger.kernel.org \
--cc=martink@posteo.de \
--cc=rientjes@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.