* [PATCH] drbd: change one-bit bitfield to be an unsigned int
@ 2014-06-14 23:07 Martin Kepplinger
2014-06-15 0:45 ` [PATCH v2] " Martin Kepplinger
0 siblings, 1 reply; 7+ messages in thread
From: Martin Kepplinger @ 2014-06-14 23:07 UTC (permalink / raw)
To: drbd-dev; +Cc: drbd-user, linux-kernel, Martin Kepplinger
The one-bit bitfield has no negative values and thus becomes an
unsigned int.
Signed-off-by: Martin Kepplinger <martink@posteo.de>
---
No idea if you take such small changes. Please ignore if not.
This applies to next-20140613 and fixes a sparse error.
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..68b4301 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)
--
1.7.10.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2] drbd: change one-bit bitfield to be an unsigned int
2014-06-14 23:07 [PATCH] drbd: change one-bit bitfield to be an unsigned int Martin Kepplinger
@ 2014-06-15 0:45 ` Martin Kepplinger
2014-06-16 9:28 ` David Rientjes
0 siblings, 1 reply; 7+ messages in thread
From: Martin Kepplinger @ 2014-06-15 0:45 UTC (permalink / raw)
To: drbd-dev; +Cc: drbd-user, linux-kernel, Martin Kepplinger
The one-bit bitfield has no negative values and thus becomes an
unsigned int.
Signed-off-by: Martin Kepplinger <martink@posteo.de>
---
Sorry for the visually ugly first patch. Take this one, if any.
thanks,
martin
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)
--
1.7.10.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2] drbd: change one-bit bitfield to be an unsigned int
2014-06-15 0:45 ` [PATCH v2] " Martin Kepplinger
@ 2014-06-16 9:28 ` David Rientjes
2014-06-17 6:54 ` Martin Kepplinger
0 siblings, 1 reply; 7+ messages in thread
From: David Rientjes @ 2014-06-16 9:28 UTC (permalink / raw)
To: Martin Kepplinger; +Cc: drbd-dev, drbd-user, linux-kernel
On Sun, 15 Jun 2014, Martin Kepplinger wrote:
> The one-bit bitfield has no negative values and thus becomes an
> unsigned int.
>
> Signed-off-by: Martin Kepplinger <martink@posteo.de>
I'm unsure what you're correcting here. These bitfields are inherently
signed, "int local:1" and "int waiting:1" are signed. They can have one
of two values, 0 or -1. So I don't know what you're saying in your
changelog.
This patch would only make sense if something is testing `local' or
`waiting' to be equal to 1. Is that what you're fixing? If so, please
specify it in the changelog with an example.
> ---
> Sorry for the visually ugly first patch. Take this one, if any.
>
> thanks,
> martin
>
>
> 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)
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2] drbd: change one-bit bitfield to be an unsigned int
2014-06-16 9:28 ` David Rientjes
@ 2014-06-17 6:54 ` Martin Kepplinger
2014-06-17 10:53 ` [Drbd-dev] " Lars Ellenberg
2014-06-17 19:46 ` David Rientjes
0 siblings, 2 replies; 7+ messages in thread
From: Martin Kepplinger @ 2014-06-17 6:54 UTC (permalink / raw)
To: rientjes; +Cc: drbd-dev, drbd-user, linux-kernel, Martin Kepplinger
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)
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)
--
1.7.10.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Drbd-dev] [PATCH v2] drbd: change one-bit bitfield to be an unsigned int
2014-06-17 6:54 ` Martin Kepplinger
@ 2014-06-17 10:53 ` Lars Ellenberg
2014-06-17 19:46 ` David Rientjes
1 sibling, 0 replies; 7+ messages in thread
From: Lars Ellenberg @ 2014-06-17 10:53 UTC (permalink / raw)
To: Martin Kepplinger; +Cc: rientjes, linux-kernel, drbd-dev
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)
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] drbd: change one-bit bitfield to be an unsigned int
2014-06-17 6:54 ` Martin Kepplinger
2014-06-17 10:53 ` [Drbd-dev] " Lars Ellenberg
@ 2014-06-17 19:46 ` David Rientjes
2014-06-18 5:50 ` Martin Kepplinger
1 sibling, 1 reply; 7+ messages in thread
From: David Rientjes @ 2014-06-17 19:46 UTC (permalink / raw)
To: Martin Kepplinger; +Cc: drbd-dev, drbd-user, linux-kernel
On Tue, 17 Jun 2014, 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.
>
Nothing is silently becoming anything, I have no idea what you're trying
to address. Is there something in drivers/block/drbd that needs this
change?
> 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.
>
There's no improvement here, you realize that the sign of one-bit
bitfields are implementation defined, correct? On what implementation
does this patch make a difference?
If you are trying to convert these to unsigned for consistency, then just
say so in the changelog and don't talk about silent changes or comparisons
to true and false that obfuscate the fact that this is just a trivial
cleanup that is based on the author's own preference rather than anything
else.
> sparse called it 'dubious' before the change.
>
> (built but untested)
>
> 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)
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] drbd: change one-bit bitfield to be an unsigned int
2014-06-17 19:46 ` David Rientjes
@ 2014-06-18 5:50 ` Martin Kepplinger
0 siblings, 0 replies; 7+ messages in thread
From: Martin Kepplinger @ 2014-06-18 5:50 UTC (permalink / raw)
To: David Rientjes; +Cc: drbd-dev, drbd-user, linux-kernel
Am 2014-06-17 21:46, schrieb David Rientjes:
> On Tue, 17 Jun 2014, 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.
>>
>
> Nothing is silently becoming anything, I have no idea what you're trying
> to address. Is there something in drivers/block/drbd that needs this
> change?
nope.
>
>> 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.
>>
>
> There's no improvement here, you realize that the sign of one-bit
> bitfields are implementation defined, correct? On what implementation
> does this patch make a difference?
>
> If you are trying to convert these to unsigned for consistency, then just
> say so in the changelog and don't talk about silent changes or comparisons
> to true and false that obfuscate the fact that this is just a trivial
> cleanup that is based on the author's own preference rather than anything
> else.
learning learning learning. in this case, why sparse calls this dubious.
This would be more appropriate on kernelnewbies or the like, sorry.
>
>> sparse called it 'dubious' before the change.
>>
>> (built but untested)
>>
>> 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)
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-06-18 5:51 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-14 23:07 [PATCH] drbd: change one-bit bitfield to be an unsigned int Martin Kepplinger
2014-06-15 0:45 ` [PATCH v2] " Martin Kepplinger
2014-06-16 9:28 ` David Rientjes
2014-06-17 6:54 ` Martin Kepplinger
2014-06-17 10:53 ` [Drbd-dev] " Lars Ellenberg
2014-06-17 19:46 ` David Rientjes
2014-06-18 5:50 ` Martin Kepplinger
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.