diff mbox

[v15,02/21] block: Define BLK_PERM_MAX

Message ID 20170426033413.17192-3-famz@redhat.com
State New
Headers show

Commit Message

Fam Zheng April 26, 2017, 3:33 a.m. UTC
This is the order of the largest possible permission.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 include/block/block.h | 2 ++
 1 file changed, 2 insertions(+)

Comments

Kevin Wolf April 26, 2017, 9:36 a.m. UTC | #1
Am 26.04.2017 um 05:33 hat Fam Zheng geschrieben:
> This is the order of the largest possible permission.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  include/block/block.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/include/block/block.h b/include/block/block.h
> index eb0565d..a798f10 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -224,6 +224,8 @@ enum {
>      BLK_PERM_ALL                = 0x1f,
>  };
>  
> +#define BLK_PERM_MAX (64 - clz64((uint64_t)BLK_PERM_ALL))

Contrary to the commit message, this is the number of permission bits in
use (i.e. one more than the largest possible permission). You're using
it correctly, though, because your loop condition is i < BLK_PERM_MAX.

This could use an updated commit message and a comment at the #define at
least. Ideally a less ambiguous name instead of the commit (because _MAX
seems to imply what the commit message currently says, not what it
really is), but I can't think of one.

Kevin
Fam Zheng April 27, 2017, 2:03 a.m. UTC | #2
On Wed, 04/26 11:36, Kevin Wolf wrote:
> Am 26.04.2017 um 05:33 hat Fam Zheng geschrieben:
> > This is the order of the largest possible permission.
> > 
> > Signed-off-by: Fam Zheng <famz@redhat.com>
> > ---
> >  include/block/block.h | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/include/block/block.h b/include/block/block.h
> > index eb0565d..a798f10 100644
> > --- a/include/block/block.h
> > +++ b/include/block/block.h
> > @@ -224,6 +224,8 @@ enum {
> >      BLK_PERM_ALL                = 0x1f,
> >  };
> >  
> > +#define BLK_PERM_MAX (64 - clz64((uint64_t)BLK_PERM_ALL))
> 
> Contrary to the commit message, this is the number of permission bits in
> use (i.e. one more than the largest possible permission). You're using
> it correctly, though, because your loop condition is i < BLK_PERM_MAX.
> 
> This could use an updated commit message and a comment at the #define at
> least. Ideally a less ambiguous name instead of the commit (because _MAX
> seems to imply what the commit message currently says, not what it
> really is), but I can't think of one.

Good point.

Given it another thought, using BLK_PERM_ALL in the loop condition is as easy.
I'll drop this patch.

Fam
diff mbox

Patch

diff --git a/include/block/block.h b/include/block/block.h
index eb0565d..a798f10 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -224,6 +224,8 @@  enum {
     BLK_PERM_ALL                = 0x1f,
 };
 
+#define BLK_PERM_MAX (64 - clz64((uint64_t)BLK_PERM_ALL))
+
 char *bdrv_perm_names(uint64_t perm);
 
 /* disk I/O throttling */