diff mbox

[v3,2/2] IB/core: Align casting method of ib_device_cap_flags enumerations to ULL

Message ID 1465230880-25953-3-git-send-email-maxg@mellanox.com (mailing list archive)
State Accepted
Headers show

Commit Message

Max Gurtovoy June 6, 2016, 4:34 p.m. UTC
Replace u64 casting to ULL.

Signed-off-by: Max Gurtovoy <maxg@mellanox.com>
---
 include/rdma/ib_verbs.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

Comments

Christoph Hellwig June 6, 2016, 5:04 p.m. UTC | #1
On Mon, Jun 06, 2016 at 07:34:40PM +0300, Max Gurtovoy wrote:
> Replace u64 casting to ULL.
> 
> Signed-off-by: Max Gurtovoy <maxg@mellanox.com>
> ---
>  include/rdma/ib_verbs.h |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
> index c97357b..7e440d4 100644
> --- a/include/rdma/ib_verbs.h
> +++ b/include/rdma/ib_verbs.h
> @@ -219,8 +219,8 @@ enum ib_device_cap_flags {
>  	IB_DEVICE_SIGNATURE_HANDOVER		= (1 << 30),
>  	IB_DEVICE_ON_DEMAND_PAGING		= (1ULL << 31),
>  	IB_DEVICE_SG_GAPS_REG			= (1ULL << 32),
> -	IB_DEVICE_VIRTUAL_FUNCTION		= ((u64)1 << 33),
> -	IB_DEVICE_RAW_SCATTER_FCS		= ((u64)1 << 34),
> +	IB_DEVICE_VIRTUAL_FUNCTION		= (1ULL << 33),
> +	IB_DEVICE_RAW_SCATTER_FCS		= (1ULL << 34),

Looks good, but can you also convert the whole enum to this
style?
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Greg KH June 6, 2016, 5:15 p.m. UTC | #2
On Mon, Jun 06, 2016 at 10:04:13AM -0700, Christoph Hellwig wrote:
> On Mon, Jun 06, 2016 at 07:34:40PM +0300, Max Gurtovoy wrote:
> > Replace u64 casting to ULL.
> > 
> > Signed-off-by: Max Gurtovoy <maxg@mellanox.com>
> > ---
> >  include/rdma/ib_verbs.h |    4 ++--
> >  1 files changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
> > index c97357b..7e440d4 100644
> > --- a/include/rdma/ib_verbs.h
> > +++ b/include/rdma/ib_verbs.h
> > @@ -219,8 +219,8 @@ enum ib_device_cap_flags {
> >  	IB_DEVICE_SIGNATURE_HANDOVER		= (1 << 30),
> >  	IB_DEVICE_ON_DEMAND_PAGING		= (1ULL << 31),
> >  	IB_DEVICE_SG_GAPS_REG			= (1ULL << 32),
> > -	IB_DEVICE_VIRTUAL_FUNCTION		= ((u64)1 << 33),
> > -	IB_DEVICE_RAW_SCATTER_FCS		= ((u64)1 << 34),
> > +	IB_DEVICE_VIRTUAL_FUNCTION		= (1ULL << 33),
> > +	IB_DEVICE_RAW_SCATTER_FCS		= (1ULL << 34),
> 
> Looks good, but can you also convert the whole enum to this
> style?

If this "convert the whole thing" is going to happen, can you use the
BIT() for this please?  I asked that last time this patch came by...

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Christoph Hellwig June 6, 2016, 5:22 p.m. UTC | #3
On Mon, Jun 06, 2016 at 10:15:43AM -0700, Greg KH wrote:
> If this "convert the whole thing" is going to happen, can you use the
> BIT() for this please?  I asked that last time this patch came by...

There is absolutely no reason for that bullshit obsfucation.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Christoph Hellwig June 6, 2016, 5:22 p.m. UTC | #4
> Later on, we will wrap all these shifts in BIT() macros as Greg suggested.

Big NAK to any use of that macro.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Greg KH June 6, 2016, 5:40 p.m. UTC | #5
On Mon, Jun 06, 2016 at 10:22:46AM -0700, hch wrote:
> > Later on, we will wrap all these shifts in BIT() macros as Greg suggested.
> 
> Big NAK to any use of that macro.

That's odd, given that most of the kernel has already converted to use
it.  You can try to keep it out of this subsystem, but you will get
patches to convert it from random people and it will wear you down over
time...

sorry,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bart Van Assche June 6, 2016, 5:45 p.m. UTC | #6
On 06/06/2016 10:40 AM, Greg KH wrote:
> On Mon, Jun 06, 2016 at 10:22:46AM -0700, hch wrote:
>>> Later on, we will wrap all these shifts in BIT() macros as Greg suggested.
>>
>> Big NAK to any use of that macro.
>
> That's odd, given that most of the kernel has already converted to use
> it.  You can try to keep it out of this subsystem, but you will get
> patches to convert it from random people and it will wear you down over
> time...

Hello Greg,

It is considered a bad programming practice to introduce macros that 
obfuscate C operations. All the BIT() macro does is a shift operation. 
Writing the shift explicitly instead of using the BIT() macro makes 
source code easier to read. So why has the BIT() macro ever been introduced?

Thanks,

Bart.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Max Gurtovoy June 6, 2016, 6:31 p.m. UTC | #7
On 6/6/2016 8:15 PM, Greg KH wrote:
> On Mon, Jun 06, 2016 at 10:04:13AM -0700, Christoph Hellwig wrote:
>> On Mon, Jun 06, 2016 at 07:34:40PM +0300, Max Gurtovoy wrote:
>>> Replace u64 casting to ULL.
>>>
>>> Signed-off-by: Max Gurtovoy <maxg@mellanox.com>
>>> ---
>>>   include/rdma/ib_verbs.h |    4 ++--
>>>   1 files changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
>>> index c97357b..7e440d4 100644
>>> --- a/include/rdma/ib_verbs.h
>>> +++ b/include/rdma/ib_verbs.h
>>> @@ -219,8 +219,8 @@ enum ib_device_cap_flags {
>>>   	IB_DEVICE_SIGNATURE_HANDOVER		= (1 << 30),
>>>   	IB_DEVICE_ON_DEMAND_PAGING		= (1ULL << 31),
>>>   	IB_DEVICE_SG_GAPS_REG			= (1ULL << 32),
>>> -	IB_DEVICE_VIRTUAL_FUNCTION		= ((u64)1 << 33),
>>> -	IB_DEVICE_RAW_SCATTER_FCS		= ((u64)1 << 34),
>>> +	IB_DEVICE_VIRTUAL_FUNCTION		= (1ULL << 33),
>>> +	IB_DEVICE_RAW_SCATTER_FCS		= (1ULL << 34),
>>
>> Looks good, but can you also convert the whole enum to this
>> style?

This was my v1 patch but many peaple didn't like it so much.
I coordinated with Doug that I'll push small patch for now and later we 
can cast the whole enum to preferable style.

>
> If this "convert the whole thing" is going to happen, can you use the
> BIT() for this please?  I asked that last time this patch came by...

It's a maintainers desicion :)

>
> thanks,
>
> greg k-h
>
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
index c97357b..7e440d4 100644
--- a/include/rdma/ib_verbs.h
+++ b/include/rdma/ib_verbs.h
@@ -219,8 +219,8 @@  enum ib_device_cap_flags {
 	IB_DEVICE_SIGNATURE_HANDOVER		= (1 << 30),
 	IB_DEVICE_ON_DEMAND_PAGING		= (1ULL << 31),
 	IB_DEVICE_SG_GAPS_REG			= (1ULL << 32),
-	IB_DEVICE_VIRTUAL_FUNCTION		= ((u64)1 << 33),
-	IB_DEVICE_RAW_SCATTER_FCS		= ((u64)1 << 34),
+	IB_DEVICE_VIRTUAL_FUNCTION		= (1ULL << 33),
+	IB_DEVICE_RAW_SCATTER_FCS		= (1ULL << 34),
 };
 
 enum ib_signature_prot_cap {