All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] xfs_db: improve argument naming in set_cur and set_iocur_type
@ 2017-06-28 15:09 Bill O'Donnell
  2017-06-28 16:40 ` Christoph Hellwig
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Bill O'Donnell @ 2017-06-28 15:09 UTC (permalink / raw)
  To: linux-xfs; +Cc: sandeen

In set_cur and set_iocur_type, the current naming for arguments
type, block number, and length are t, d, and c, respectively.
Replace these with more intuitive and descriptive names:
type, blknum, and len. Additionally remove extra blank line
in io.c.

Signed-off-by: Bill O'Donnell <billodo@redhat.com>
---
 db/io.c | 31 +++++++++++++++----------------
 db/io.h |  6 +++---
 2 files changed, 18 insertions(+), 19 deletions(-)

diff --git a/db/io.c b/db/io.c
index 9918a51d..45cbf3be 100644
--- a/db/io.c
+++ b/db/io.c
@@ -535,9 +535,9 @@ write_cur(void)
 
 void
 set_cur(
-	const typ_t	*t,
-	__int64_t	d,
-	int		c,
+	const typ_t	*type,
+	__int64_t	blknum,
+	int		len,
 	int		ring_flag,
 	bbmap_t		*bbmap)
 {
@@ -545,14 +545,13 @@ set_cur(
 	xfs_ino_t	dirino;
 	xfs_ino_t	ino;
 	__uint16_t	mode;
-	const struct xfs_buf_ops *ops = t ? t->bops : NULL;
+	const struct xfs_buf_ops *ops = type ? type->bops : NULL;
 
 	if (iocur_sp < 0) {
 		dbprintf(_("set_cur no stack element to set\n"));
 		return;
 	}
 
-
 	ino = iocur_top->ino;
 	dirino = iocur_top->dirino;
 	mode = iocur_top->mode;
@@ -562,7 +561,7 @@ set_cur(
 	if (bbmap) {
 #ifdef DEBUG_BBMAP
 		int i;
-		printf(_("xfs_db got a bbmap for %lld\n"), (long long)d);
+		printf(_("xfs_db got a bbmap for %lld\n"), (long long)blknum);
 		printf(_("\tblock map"));
 		for (i = 0; i < bbmap->nmaps; i++)
 			printf(" %lld:%d", (long long)bbmap->b[i].bm_bn,
@@ -576,7 +575,7 @@ set_cur(
 		bp = libxfs_readbuf_map(mp->m_ddev_targp, bbmap->b,
 					bbmap->nmaps, 0, ops);
 	} else {
-		bp = libxfs_readbuf(mp->m_ddev_targp, d, c, 0, ops);
+		bp = libxfs_readbuf(mp->m_ddev_targp, blknum, len, 0, ops);
 		iocur_top->bbmap = NULL;
 	}
 
@@ -592,13 +591,13 @@ set_cur(
 	if (!ops)
 		bp->b_flags |= LIBXFS_B_UNCHECKED;
 
-	iocur_top->bb = d;
-	iocur_top->blen = c;
+	iocur_top->bb = blknum;
+	iocur_top->blen = len;
 	iocur_top->boff = 0;
 	iocur_top->data = iocur_top->buf;
-	iocur_top->len = BBTOB(c);
-	iocur_top->off = d << BBSHIFT;
-	iocur_top->typ = cur_typ = t;
+	iocur_top->len = BBTOB(len);
+	iocur_top->off = blknum << BBSHIFT;
+	iocur_top->typ = cur_typ = type;
 	iocur_top->ino = ino;
 	iocur_top->dirino = dirino;
 	iocur_top->mode = mode;
@@ -612,16 +611,16 @@ set_cur(
 
 void
 set_iocur_type(
-	const typ_t	*t)
+	const typ_t	*type)
 {
 	struct xfs_buf	*bp = iocur_top->bp;
 
-	iocur_top->typ = t;
+	iocur_top->typ = type;
 
 	/* verify the buffer if the type has one. */
 	if (!bp)
 		return;
-	if (!t->bops) {
+	if (!type->bops) {
 		bp->b_ops = NULL;
 		bp->b_flags |= LIBXFS_B_UNCHECKED;
 		return;
@@ -629,7 +628,7 @@ set_iocur_type(
 	if (!(bp->b_flags & LIBXFS_B_UPTODATE))
 		return;
 	bp->b_error = 0;
-	bp->b_ops = t->bops;
+	bp->b_ops = type->bops;
 	bp->b_ops->verify_read(bp);
 	bp->b_flags &= ~LIBXFS_B_UNCHECKED;
 }
diff --git a/db/io.h b/db/io.h
index b415b82d..ebf51f1d 100644
--- a/db/io.h
+++ b/db/io.h
@@ -59,10 +59,10 @@ extern void	print_iocur(char *tag, iocur_t *ioc);
 extern void	push_cur(void);
 extern int	read_buf(__int64_t daddr, int count, void *bufp);
 extern void     write_cur(void);
-extern void	set_cur(const struct typ *t, __int64_t d, int c, int ring_add,
-			bbmap_t *bbmap);
+extern void	set_cur(const struct typ *type, __int64_t blknum,
+			int len, int ring_add, bbmap_t *bbmap);
 extern void     ring_add(void);
-extern void	set_iocur_type(const struct typ *t);
+extern void	set_iocur_type(const struct typ *type);
 extern void	xfs_dummy_verify(struct xfs_buf *bp);
 extern void	xfs_verify_recalc_inode_crc(struct xfs_buf *bp);
 extern void	xfs_verify_recalc_dquot_crc(struct xfs_buf *bp);
-- 
2.13.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] xfs_db: improve argument naming in set_cur and set_iocur_type
  2017-06-28 15:09 [PATCH] xfs_db: improve argument naming in set_cur and set_iocur_type Bill O'Donnell
@ 2017-06-28 16:40 ` Christoph Hellwig
  2017-06-28 16:48 ` Darrick J. Wong
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Christoph Hellwig @ 2017-06-28 16:40 UTC (permalink / raw)
  To: Bill O'Donnell; +Cc: linux-xfs, sandeen

Looks fine,

Reviewed-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] xfs_db: improve argument naming in set_cur and set_iocur_type
  2017-06-28 15:09 [PATCH] xfs_db: improve argument naming in set_cur and set_iocur_type Bill O'Donnell
  2017-06-28 16:40 ` Christoph Hellwig
@ 2017-06-28 16:48 ` Darrick J. Wong
  2017-06-28 18:04   ` Bill O'Donnell
  2017-06-28 18:38 ` Eric Sandeen
  2017-06-28 19:25 ` [PATCH v2] " Bill O'Donnell
  3 siblings, 1 reply; 7+ messages in thread
From: Darrick J. Wong @ 2017-06-28 16:48 UTC (permalink / raw)
  To: Bill O'Donnell; +Cc: linux-xfs, sandeen

On Wed, Jun 28, 2017 at 10:09:13AM -0500, Bill O'Donnell wrote:
> In set_cur and set_iocur_type, the current naming for arguments
> type, block number, and length are t, d, and c, respectively.
> Replace these with more intuitive and descriptive names:

Yay!

> type, blknum, and len. Additionally remove extra blank line
> in io.c.
> 
> Signed-off-by: Bill O'Donnell <billodo@redhat.com>
> ---
>  db/io.c | 31 +++++++++++++++----------------
>  db/io.h |  6 +++---
>  2 files changed, 18 insertions(+), 19 deletions(-)
> 
> diff --git a/db/io.c b/db/io.c
> index 9918a51d..45cbf3be 100644
> --- a/db/io.c
> +++ b/db/io.c
> @@ -535,9 +535,9 @@ write_cur(void)
>  
>  void
>  set_cur(
> -	const typ_t	*t,
> -	__int64_t	d,
> -	int		c,
> +	const typ_t	*type,
> +	__int64_t	blknum,

/me wonders if this ought to be "xfs_daddr_t blknum" since that's what
libxfs_readbuf takes?

Other than that,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

> +	int		len,

This parameter /ought/ to be unsigned int, because a negative length
makes no sense and xfs_bufkey and xfs_buf have unsigned int lengths.
OTOH the libxfs/rdwr.c functions take signed int, so all that can be a
separate cleanup patch to fix all that.

--D

>  	int		ring_flag,
>  	bbmap_t		*bbmap)
>  {
> @@ -545,14 +545,13 @@ set_cur(
>  	xfs_ino_t	dirino;
>  	xfs_ino_t	ino;
>  	__uint16_t	mode;
> -	const struct xfs_buf_ops *ops = t ? t->bops : NULL;
> +	const struct xfs_buf_ops *ops = type ? type->bops : NULL;
>  
>  	if (iocur_sp < 0) {
>  		dbprintf(_("set_cur no stack element to set\n"));
>  		return;
>  	}
>  
> -
>  	ino = iocur_top->ino;
>  	dirino = iocur_top->dirino;
>  	mode = iocur_top->mode;
> @@ -562,7 +561,7 @@ set_cur(
>  	if (bbmap) {
>  #ifdef DEBUG_BBMAP
>  		int i;
> -		printf(_("xfs_db got a bbmap for %lld\n"), (long long)d);
> +		printf(_("xfs_db got a bbmap for %lld\n"), (long long)blknum);
>  		printf(_("\tblock map"));
>  		for (i = 0; i < bbmap->nmaps; i++)
>  			printf(" %lld:%d", (long long)bbmap->b[i].bm_bn,
> @@ -576,7 +575,7 @@ set_cur(
>  		bp = libxfs_readbuf_map(mp->m_ddev_targp, bbmap->b,
>  					bbmap->nmaps, 0, ops);
>  	} else {
> -		bp = libxfs_readbuf(mp->m_ddev_targp, d, c, 0, ops);
> +		bp = libxfs_readbuf(mp->m_ddev_targp, blknum, len, 0, ops);
>  		iocur_top->bbmap = NULL;
>  	}
>  
> @@ -592,13 +591,13 @@ set_cur(
>  	if (!ops)
>  		bp->b_flags |= LIBXFS_B_UNCHECKED;
>  
> -	iocur_top->bb = d;
> -	iocur_top->blen = c;
> +	iocur_top->bb = blknum;
> +	iocur_top->blen = len;
>  	iocur_top->boff = 0;
>  	iocur_top->data = iocur_top->buf;
> -	iocur_top->len = BBTOB(c);
> -	iocur_top->off = d << BBSHIFT;
> -	iocur_top->typ = cur_typ = t;
> +	iocur_top->len = BBTOB(len);
> +	iocur_top->off = blknum << BBSHIFT;
> +	iocur_top->typ = cur_typ = type;
>  	iocur_top->ino = ino;
>  	iocur_top->dirino = dirino;
>  	iocur_top->mode = mode;
> @@ -612,16 +611,16 @@ set_cur(
>  
>  void
>  set_iocur_type(
> -	const typ_t	*t)
> +	const typ_t	*type)
>  {
>  	struct xfs_buf	*bp = iocur_top->bp;
>  
> -	iocur_top->typ = t;
> +	iocur_top->typ = type;
>  
>  	/* verify the buffer if the type has one. */
>  	if (!bp)
>  		return;
> -	if (!t->bops) {
> +	if (!type->bops) {
>  		bp->b_ops = NULL;
>  		bp->b_flags |= LIBXFS_B_UNCHECKED;
>  		return;
> @@ -629,7 +628,7 @@ set_iocur_type(
>  	if (!(bp->b_flags & LIBXFS_B_UPTODATE))
>  		return;
>  	bp->b_error = 0;
> -	bp->b_ops = t->bops;
> +	bp->b_ops = type->bops;
>  	bp->b_ops->verify_read(bp);
>  	bp->b_flags &= ~LIBXFS_B_UNCHECKED;
>  }
> diff --git a/db/io.h b/db/io.h
> index b415b82d..ebf51f1d 100644
> --- a/db/io.h
> +++ b/db/io.h
> @@ -59,10 +59,10 @@ extern void	print_iocur(char *tag, iocur_t *ioc);
>  extern void	push_cur(void);
>  extern int	read_buf(__int64_t daddr, int count, void *bufp);
>  extern void     write_cur(void);
> -extern void	set_cur(const struct typ *t, __int64_t d, int c, int ring_add,
> -			bbmap_t *bbmap);
> +extern void	set_cur(const struct typ *type, __int64_t blknum,
> +			int len, int ring_add, bbmap_t *bbmap);
>  extern void     ring_add(void);
> -extern void	set_iocur_type(const struct typ *t);
> +extern void	set_iocur_type(const struct typ *type);
>  extern void	xfs_dummy_verify(struct xfs_buf *bp);
>  extern void	xfs_verify_recalc_inode_crc(struct xfs_buf *bp);
>  extern void	xfs_verify_recalc_dquot_crc(struct xfs_buf *bp);
> -- 
> 2.13.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" 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] 7+ messages in thread

* Re: [PATCH] xfs_db: improve argument naming in set_cur and set_iocur_type
  2017-06-28 16:48 ` Darrick J. Wong
@ 2017-06-28 18:04   ` Bill O'Donnell
  0 siblings, 0 replies; 7+ messages in thread
From: Bill O'Donnell @ 2017-06-28 18:04 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs, sandeen

On Wed, Jun 28, 2017 at 09:48:18AM -0700, Darrick J. Wong wrote:
> On Wed, Jun 28, 2017 at 10:09:13AM -0500, Bill O'Donnell wrote:
> > In set_cur and set_iocur_type, the current naming for arguments
> > type, block number, and length are t, d, and c, respectively.
> > Replace these with more intuitive and descriptive names:
> 
> Yay!
> 
> > type, blknum, and len. Additionally remove extra blank line
> > in io.c.
> > 
> > Signed-off-by: Bill O'Donnell <billodo@redhat.com>
> > ---
> >  db/io.c | 31 +++++++++++++++----------------
> >  db/io.h |  6 +++---
> >  2 files changed, 18 insertions(+), 19 deletions(-)
> > 
> > diff --git a/db/io.c b/db/io.c
> > index 9918a51d..45cbf3be 100644
> > --- a/db/io.c
> > +++ b/db/io.c
> > @@ -535,9 +535,9 @@ write_cur(void)
> >  
> >  void
> >  set_cur(
> > -	const typ_t	*t,
> > -	__int64_t	d,
> > -	int		c,
> > +	const typ_t	*type,
> > +	__int64_t	blknum,
> 
> /me wonders if this ought to be "xfs_daddr_t blknum" since that's what
> libxfs_readbuf takes?

True. xfs_daddr_t is typedef to __int64_t, and it might make good sense
to become consistent with libxfs_readbuf arg types.


> 
> Other than that,
> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
> 
> > +	int		len,
> 
> This parameter /ought/ to be unsigned int, because a negative length
> makes no sense and xfs_bufkey and xfs_buf have unsigned int lengths.
> OTOH the libxfs/rdwr.c functions take signed int, so all that can be a
> separate cleanup patch to fix all that.
> 
> --D
> 
> >  	int		ring_flag,
> >  	bbmap_t		*bbmap)
> >  {
> > @@ -545,14 +545,13 @@ set_cur(
> >  	xfs_ino_t	dirino;
> >  	xfs_ino_t	ino;
> >  	__uint16_t	mode;
> > -	const struct xfs_buf_ops *ops = t ? t->bops : NULL;
> > +	const struct xfs_buf_ops *ops = type ? type->bops : NULL;
> >  
> >  	if (iocur_sp < 0) {
> >  		dbprintf(_("set_cur no stack element to set\n"));
> >  		return;
> >  	}
> >  
> > -
> >  	ino = iocur_top->ino;
> >  	dirino = iocur_top->dirino;
> >  	mode = iocur_top->mode;
> > @@ -562,7 +561,7 @@ set_cur(
> >  	if (bbmap) {
> >  #ifdef DEBUG_BBMAP
> >  		int i;
> > -		printf(_("xfs_db got a bbmap for %lld\n"), (long long)d);
> > +		printf(_("xfs_db got a bbmap for %lld\n"), (long long)blknum);
> >  		printf(_("\tblock map"));
> >  		for (i = 0; i < bbmap->nmaps; i++)
> >  			printf(" %lld:%d", (long long)bbmap->b[i].bm_bn,
> > @@ -576,7 +575,7 @@ set_cur(
> >  		bp = libxfs_readbuf_map(mp->m_ddev_targp, bbmap->b,
> >  					bbmap->nmaps, 0, ops);
> >  	} else {
> > -		bp = libxfs_readbuf(mp->m_ddev_targp, d, c, 0, ops);
> > +		bp = libxfs_readbuf(mp->m_ddev_targp, blknum, len, 0, ops);
> >  		iocur_top->bbmap = NULL;
> >  	}
> >  
> > @@ -592,13 +591,13 @@ set_cur(
> >  	if (!ops)
> >  		bp->b_flags |= LIBXFS_B_UNCHECKED;
> >  
> > -	iocur_top->bb = d;
> > -	iocur_top->blen = c;
> > +	iocur_top->bb = blknum;
> > +	iocur_top->blen = len;
> >  	iocur_top->boff = 0;
> >  	iocur_top->data = iocur_top->buf;
> > -	iocur_top->len = BBTOB(c);
> > -	iocur_top->off = d << BBSHIFT;
> > -	iocur_top->typ = cur_typ = t;
> > +	iocur_top->len = BBTOB(len);
> > +	iocur_top->off = blknum << BBSHIFT;
> > +	iocur_top->typ = cur_typ = type;
> >  	iocur_top->ino = ino;
> >  	iocur_top->dirino = dirino;
> >  	iocur_top->mode = mode;
> > @@ -612,16 +611,16 @@ set_cur(
> >  
> >  void
> >  set_iocur_type(
> > -	const typ_t	*t)
> > +	const typ_t	*type)
> >  {
> >  	struct xfs_buf	*bp = iocur_top->bp;
> >  
> > -	iocur_top->typ = t;
> > +	iocur_top->typ = type;
> >  
> >  	/* verify the buffer if the type has one. */
> >  	if (!bp)
> >  		return;
> > -	if (!t->bops) {
> > +	if (!type->bops) {
> >  		bp->b_ops = NULL;
> >  		bp->b_flags |= LIBXFS_B_UNCHECKED;
> >  		return;
> > @@ -629,7 +628,7 @@ set_iocur_type(
> >  	if (!(bp->b_flags & LIBXFS_B_UPTODATE))
> >  		return;
> >  	bp->b_error = 0;
> > -	bp->b_ops = t->bops;
> > +	bp->b_ops = type->bops;
> >  	bp->b_ops->verify_read(bp);
> >  	bp->b_flags &= ~LIBXFS_B_UNCHECKED;
> >  }
> > diff --git a/db/io.h b/db/io.h
> > index b415b82d..ebf51f1d 100644
> > --- a/db/io.h
> > +++ b/db/io.h
> > @@ -59,10 +59,10 @@ extern void	print_iocur(char *tag, iocur_t *ioc);
> >  extern void	push_cur(void);
> >  extern int	read_buf(__int64_t daddr, int count, void *bufp);
> >  extern void     write_cur(void);
> > -extern void	set_cur(const struct typ *t, __int64_t d, int c, int ring_add,
> > -			bbmap_t *bbmap);
> > +extern void	set_cur(const struct typ *type, __int64_t blknum,
> > +			int len, int ring_add, bbmap_t *bbmap);
> >  extern void     ring_add(void);
> > -extern void	set_iocur_type(const struct typ *t);
> > +extern void	set_iocur_type(const struct typ *type);
> >  extern void	xfs_dummy_verify(struct xfs_buf *bp);
> >  extern void	xfs_verify_recalc_inode_crc(struct xfs_buf *bp);
> >  extern void	xfs_verify_recalc_dquot_crc(struct xfs_buf *bp);
> > -- 
> > 2.13.0
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-xfs" 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] 7+ messages in thread

* Re: [PATCH] xfs_db: improve argument naming in set_cur and set_iocur_type
  2017-06-28 15:09 [PATCH] xfs_db: improve argument naming in set_cur and set_iocur_type Bill O'Donnell
  2017-06-28 16:40 ` Christoph Hellwig
  2017-06-28 16:48 ` Darrick J. Wong
@ 2017-06-28 18:38 ` Eric Sandeen
  2017-06-28 18:42   ` Bill O'Donnell
  2017-06-28 19:25 ` [PATCH v2] " Bill O'Donnell
  3 siblings, 1 reply; 7+ messages in thread
From: Eric Sandeen @ 2017-06-28 18:38 UTC (permalink / raw)
  To: Bill O'Donnell, linux-xfs

On 6/28/17 10:09 AM, Bill O'Donnell wrote:
> In set_cur and set_iocur_type, the current naming for arguments
> type, block number, and length are t, d, and c, respectively.
> Replace these with more intuitive and descriptive names:
> type, blknum, and len. Additionally remove extra blank line
> in io.c.
> 
> Signed-off-by: Bill O'Donnell <billodo@redhat.com>

Could you resend this, rebased on top of the patch you just sent
for set_iocur_type() please?

(also: watch out for long lines ...)

That'll give you a chance to change the type pointed out earlier,
as well.

Thanks,
-Eric

> ---
>  db/io.c | 31 +++++++++++++++----------------
>  db/io.h |  6 +++---
>  2 files changed, 18 insertions(+), 19 deletions(-)
> 
> diff --git a/db/io.c b/db/io.c
> index 9918a51d..45cbf3be 100644
> --- a/db/io.c
> +++ b/db/io.c
> @@ -535,9 +535,9 @@ write_cur(void)
>  
>  void
>  set_cur(
> -	const typ_t	*t,
> -	__int64_t	d,
> -	int		c,
> +	const typ_t	*type,
> +	__int64_t	blknum,
> +	int		len,
>  	int		ring_flag,
>  	bbmap_t		*bbmap)
>  {
> @@ -545,14 +545,13 @@ set_cur(
>  	xfs_ino_t	dirino;
>  	xfs_ino_t	ino;
>  	__uint16_t	mode;
> -	const struct xfs_buf_ops *ops = t ? t->bops : NULL;
> +	const struct xfs_buf_ops *ops = type ? type->bops : NULL;
>  
>  	if (iocur_sp < 0) {
>  		dbprintf(_("set_cur no stack element to set\n"));
>  		return;
>  	}
>  
> -
>  	ino = iocur_top->ino;
>  	dirino = iocur_top->dirino;
>  	mode = iocur_top->mode;
> @@ -562,7 +561,7 @@ set_cur(
>  	if (bbmap) {
>  #ifdef DEBUG_BBMAP
>  		int i;
> -		printf(_("xfs_db got a bbmap for %lld\n"), (long long)d);
> +		printf(_("xfs_db got a bbmap for %lld\n"), (long long)blknum);
>  		printf(_("\tblock map"));
>  		for (i = 0; i < bbmap->nmaps; i++)
>  			printf(" %lld:%d", (long long)bbmap->b[i].bm_bn,
> @@ -576,7 +575,7 @@ set_cur(
>  		bp = libxfs_readbuf_map(mp->m_ddev_targp, bbmap->b,
>  					bbmap->nmaps, 0, ops);
>  	} else {
> -		bp = libxfs_readbuf(mp->m_ddev_targp, d, c, 0, ops);
> +		bp = libxfs_readbuf(mp->m_ddev_targp, blknum, len, 0, ops);
>  		iocur_top->bbmap = NULL;
>  	}
>  
> @@ -592,13 +591,13 @@ set_cur(
>  	if (!ops)
>  		bp->b_flags |= LIBXFS_B_UNCHECKED;
>  
> -	iocur_top->bb = d;
> -	iocur_top->blen = c;
> +	iocur_top->bb = blknum;
> +	iocur_top->blen = len;
>  	iocur_top->boff = 0;
>  	iocur_top->data = iocur_top->buf;
> -	iocur_top->len = BBTOB(c);
> -	iocur_top->off = d << BBSHIFT;
> -	iocur_top->typ = cur_typ = t;
> +	iocur_top->len = BBTOB(len);
> +	iocur_top->off = blknum << BBSHIFT;
> +	iocur_top->typ = cur_typ = type;
>  	iocur_top->ino = ino;
>  	iocur_top->dirino = dirino;
>  	iocur_top->mode = mode;
> @@ -612,16 +611,16 @@ set_cur(
>  
>  void
>  set_iocur_type(
> -	const typ_t	*t)
> +	const typ_t	*type)
>  {
>  	struct xfs_buf	*bp = iocur_top->bp;
>  
> -	iocur_top->typ = t;
> +	iocur_top->typ = type;
>  
>  	/* verify the buffer if the type has one. */
>  	if (!bp)
>  		return;
> -	if (!t->bops) {
> +	if (!type->bops) {
>  		bp->b_ops = NULL;
>  		bp->b_flags |= LIBXFS_B_UNCHECKED;
>  		return;
> @@ -629,7 +628,7 @@ set_iocur_type(
>  	if (!(bp->b_flags & LIBXFS_B_UPTODATE))
>  		return;
>  	bp->b_error = 0;
> -	bp->b_ops = t->bops;
> +	bp->b_ops = type->bops;
>  	bp->b_ops->verify_read(bp);
>  	bp->b_flags &= ~LIBXFS_B_UNCHECKED;
>  }
> diff --git a/db/io.h b/db/io.h
> index b415b82d..ebf51f1d 100644
> --- a/db/io.h
> +++ b/db/io.h
> @@ -59,10 +59,10 @@ extern void	print_iocur(char *tag, iocur_t *ioc);
>  extern void	push_cur(void);
>  extern int	read_buf(__int64_t daddr, int count, void *bufp);
>  extern void     write_cur(void);
> -extern void	set_cur(const struct typ *t, __int64_t d, int c, int ring_add,
> -			bbmap_t *bbmap);
> +extern void	set_cur(const struct typ *type, __int64_t blknum,
> +			int len, int ring_add, bbmap_t *bbmap);
>  extern void     ring_add(void);
> -extern void	set_iocur_type(const struct typ *t);
> +extern void	set_iocur_type(const struct typ *type);
>  extern void	xfs_dummy_verify(struct xfs_buf *bp);
>  extern void	xfs_verify_recalc_inode_crc(struct xfs_buf *bp);
>  extern void	xfs_verify_recalc_dquot_crc(struct xfs_buf *bp);
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] xfs_db: improve argument naming in set_cur and set_iocur_type
  2017-06-28 18:38 ` Eric Sandeen
@ 2017-06-28 18:42   ` Bill O'Donnell
  0 siblings, 0 replies; 7+ messages in thread
From: Bill O'Donnell @ 2017-06-28 18:42 UTC (permalink / raw)
  To: Eric Sandeen; +Cc: linux-xfs

On Wed, Jun 28, 2017 at 01:38:35PM -0500, Eric Sandeen wrote:
> On 6/28/17 10:09 AM, Bill O'Donnell wrote:
> > In set_cur and set_iocur_type, the current naming for arguments
> > type, block number, and length are t, d, and c, respectively.
> > Replace these with more intuitive and descriptive names:
> > type, blknum, and len. Additionally remove extra blank line
> > in io.c.
> > 
> > Signed-off-by: Bill O'Donnell <billodo@redhat.com>
> 
> Could you resend this, rebased on top of the patch you just sent
> for set_iocur_type() please?

Yes, I'll do that.
Thanks-
Bill

> 
> (also: watch out for long lines ...)
> 
> That'll give you a chance to change the type pointed out earlier,
> as well.
> 
> Thanks,
> -Eric
> 
> > ---
> >  db/io.c | 31 +++++++++++++++----------------
> >  db/io.h |  6 +++---
> >  2 files changed, 18 insertions(+), 19 deletions(-)
> > 
> > diff --git a/db/io.c b/db/io.c
> > index 9918a51d..45cbf3be 100644
> > --- a/db/io.c
> > +++ b/db/io.c
> > @@ -535,9 +535,9 @@ write_cur(void)
> >  
> >  void
> >  set_cur(
> > -	const typ_t	*t,
> > -	__int64_t	d,
> > -	int		c,
> > +	const typ_t	*type,
> > +	__int64_t	blknum,
> > +	int		len,
> >  	int		ring_flag,
> >  	bbmap_t		*bbmap)
> >  {
> > @@ -545,14 +545,13 @@ set_cur(
> >  	xfs_ino_t	dirino;
> >  	xfs_ino_t	ino;
> >  	__uint16_t	mode;
> > -	const struct xfs_buf_ops *ops = t ? t->bops : NULL;
> > +	const struct xfs_buf_ops *ops = type ? type->bops : NULL;
> >  
> >  	if (iocur_sp < 0) {
> >  		dbprintf(_("set_cur no stack element to set\n"));
> >  		return;
> >  	}
> >  
> > -
> >  	ino = iocur_top->ino;
> >  	dirino = iocur_top->dirino;
> >  	mode = iocur_top->mode;
> > @@ -562,7 +561,7 @@ set_cur(
> >  	if (bbmap) {
> >  #ifdef DEBUG_BBMAP
> >  		int i;
> > -		printf(_("xfs_db got a bbmap for %lld\n"), (long long)d);
> > +		printf(_("xfs_db got a bbmap for %lld\n"), (long long)blknum);
> >  		printf(_("\tblock map"));
> >  		for (i = 0; i < bbmap->nmaps; i++)
> >  			printf(" %lld:%d", (long long)bbmap->b[i].bm_bn,
> > @@ -576,7 +575,7 @@ set_cur(
> >  		bp = libxfs_readbuf_map(mp->m_ddev_targp, bbmap->b,
> >  					bbmap->nmaps, 0, ops);
> >  	} else {
> > -		bp = libxfs_readbuf(mp->m_ddev_targp, d, c, 0, ops);
> > +		bp = libxfs_readbuf(mp->m_ddev_targp, blknum, len, 0, ops);
> >  		iocur_top->bbmap = NULL;
> >  	}
> >  
> > @@ -592,13 +591,13 @@ set_cur(
> >  	if (!ops)
> >  		bp->b_flags |= LIBXFS_B_UNCHECKED;
> >  
> > -	iocur_top->bb = d;
> > -	iocur_top->blen = c;
> > +	iocur_top->bb = blknum;
> > +	iocur_top->blen = len;
> >  	iocur_top->boff = 0;
> >  	iocur_top->data = iocur_top->buf;
> > -	iocur_top->len = BBTOB(c);
> > -	iocur_top->off = d << BBSHIFT;
> > -	iocur_top->typ = cur_typ = t;
> > +	iocur_top->len = BBTOB(len);
> > +	iocur_top->off = blknum << BBSHIFT;
> > +	iocur_top->typ = cur_typ = type;
> >  	iocur_top->ino = ino;
> >  	iocur_top->dirino = dirino;
> >  	iocur_top->mode = mode;
> > @@ -612,16 +611,16 @@ set_cur(
> >  
> >  void
> >  set_iocur_type(
> > -	const typ_t	*t)
> > +	const typ_t	*type)
> >  {
> >  	struct xfs_buf	*bp = iocur_top->bp;
> >  
> > -	iocur_top->typ = t;
> > +	iocur_top->typ = type;
> >  
> >  	/* verify the buffer if the type has one. */
> >  	if (!bp)
> >  		return;
> > -	if (!t->bops) {
> > +	if (!type->bops) {
> >  		bp->b_ops = NULL;
> >  		bp->b_flags |= LIBXFS_B_UNCHECKED;
> >  		return;
> > @@ -629,7 +628,7 @@ set_iocur_type(
> >  	if (!(bp->b_flags & LIBXFS_B_UPTODATE))
> >  		return;
> >  	bp->b_error = 0;
> > -	bp->b_ops = t->bops;
> > +	bp->b_ops = type->bops;
> >  	bp->b_ops->verify_read(bp);
> >  	bp->b_flags &= ~LIBXFS_B_UNCHECKED;
> >  }
> > diff --git a/db/io.h b/db/io.h
> > index b415b82d..ebf51f1d 100644
> > --- a/db/io.h
> > +++ b/db/io.h
> > @@ -59,10 +59,10 @@ extern void	print_iocur(char *tag, iocur_t *ioc);
> >  extern void	push_cur(void);
> >  extern int	read_buf(__int64_t daddr, int count, void *bufp);
> >  extern void     write_cur(void);
> > -extern void	set_cur(const struct typ *t, __int64_t d, int c, int ring_add,
> > -			bbmap_t *bbmap);
> > +extern void	set_cur(const struct typ *type, __int64_t blknum,
> > +			int len, int ring_add, bbmap_t *bbmap);
> >  extern void     ring_add(void);
> > -extern void	set_iocur_type(const struct typ *t);
> > +extern void	set_iocur_type(const struct typ *type);
> >  extern void	xfs_dummy_verify(struct xfs_buf *bp);
> >  extern void	xfs_verify_recalc_inode_crc(struct xfs_buf *bp);
> >  extern void	xfs_verify_recalc_dquot_crc(struct xfs_buf *bp);
> > 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" 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] 7+ messages in thread

* [PATCH v2] xfs_db: improve argument naming in set_cur and set_iocur_type
  2017-06-28 15:09 [PATCH] xfs_db: improve argument naming in set_cur and set_iocur_type Bill O'Donnell
                   ` (2 preceding siblings ...)
  2017-06-28 18:38 ` Eric Sandeen
@ 2017-06-28 19:25 ` Bill O'Donnell
  3 siblings, 0 replies; 7+ messages in thread
From: Bill O'Donnell @ 2017-06-28 19:25 UTC (permalink / raw)
  To: linux-xfs

In set_cur and set_iocur_type, the current naming for arguments
type, block number, and length are t, d, and c, respectively.
Replace these with more intuitive and descriptive names:
type, blknum, and len. Fix type of blknum (xfs_daddr_t) to be
consistent with that of libxfs_readbuf where it's used.
Additionally remove extra blank line in io.c.

Signed-off-by: Bill O'Donnell <billodo@redhat.com>
---
v2: Fix type of blknum (xfs_daddr_t) to be consistent with that of
    libxfs_readbuf. Rebase this patch on recently submitted
    xfs_db patch touching the same file.
    (https://www.spinics.net/lists/linux-xfs/msg07909.html)


 db/io.c | 38 +++++++++++++++++++-------------------
 db/io.h |  6 +++---
 2 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/db/io.c b/db/io.c
index b97b710d..9a893932 100644
--- a/db/io.c
+++ b/db/io.c
@@ -536,9 +536,9 @@ write_cur(void)
 
 void
 set_cur(
-	const typ_t	*t,
-	__int64_t	d,
-	int		c,
+	const typ_t	*type,
+	xfs_daddr_t	blknum,
+	int		len,
 	int		ring_flag,
 	bbmap_t		*bbmap)
 {
@@ -546,14 +546,13 @@ set_cur(
 	xfs_ino_t	dirino;
 	xfs_ino_t	ino;
 	__uint16_t	mode;
-	const struct xfs_buf_ops *ops = t ? t->bops : NULL;
+	const struct xfs_buf_ops *ops = type ? type->bops : NULL;
 
 	if (iocur_sp < 0) {
 		dbprintf(_("set_cur no stack element to set\n"));
 		return;
 	}
 
-
 	ino = iocur_top->ino;
 	dirino = iocur_top->dirino;
 	mode = iocur_top->mode;
@@ -563,7 +562,7 @@ set_cur(
 	if (bbmap) {
 #ifdef DEBUG_BBMAP
 		int i;
-		printf(_("xfs_db got a bbmap for %lld\n"), (long long)d);
+		printf(_("xfs_db got a bbmap for %lld\n"), (long long)blknum);
 		printf(_("\tblock map"));
 		for (i = 0; i < bbmap->nmaps; i++)
 			printf(" %lld:%d", (long long)bbmap->b[i].bm_bn,
@@ -577,7 +576,7 @@ set_cur(
 		bp = libxfs_readbuf_map(mp->m_ddev_targp, bbmap->b,
 					bbmap->nmaps, 0, ops);
 	} else {
-		bp = libxfs_readbuf(mp->m_ddev_targp, d, c, 0, ops);
+		bp = libxfs_readbuf(mp->m_ddev_targp, blknum, len, 0, ops);
 		iocur_top->bbmap = NULL;
 	}
 
@@ -593,13 +592,13 @@ set_cur(
 	if (!ops)
 		bp->b_flags |= LIBXFS_B_UNCHECKED;
 
-	iocur_top->bb = d;
-	iocur_top->blen = c;
+	iocur_top->bb = blknum;
+	iocur_top->blen = len;
 	iocur_top->boff = 0;
 	iocur_top->data = iocur_top->buf;
-	iocur_top->len = BBTOB(c);
-	iocur_top->off = d << BBSHIFT;
-	iocur_top->typ = cur_typ = t;
+	iocur_top->len = BBTOB(len);
+	iocur_top->off = blknum << BBSHIFT;
+	iocur_top->typ = cur_typ = type;
 	iocur_top->ino = ino;
 	iocur_top->dirino = dirino;
 	iocur_top->mode = mode;
@@ -613,22 +612,23 @@ set_cur(
 
 void
 set_iocur_type(
-	const typ_t	*t)
+	const typ_t	*type)
 {
 	struct xfs_buf	*bp = iocur_top->bp;
 	int bb_count;
 
 	/* adjust cursor for types that contain fields */
-	if (t->fields) {
-		bb_count = BTOBB(byteize(fsize(t->fields, iocur_top->data, 0, 0)));
-		set_cur(t, iocur_top->bb, bb_count, DB_RING_IGN, NULL);
+	if (type->fields) {
+		bb_count = BTOBB(byteize(fsize(type->fields,
+					       iocur_top->data, 0, 0)));
+		set_cur(type, iocur_top->bb, bb_count, DB_RING_IGN, NULL);
 	}
-	iocur_top->typ = t;
+	iocur_top->typ = type;
 
 	/* verify the buffer if the type has one. */
 	if (!bp)
 		return;
-	if (!t->bops) {
+	if (!type->bops) {
 		bp->b_ops = NULL;
 		bp->b_flags |= LIBXFS_B_UNCHECKED;
 		return;
@@ -636,7 +636,7 @@ set_iocur_type(
 	if (!(bp->b_flags & LIBXFS_B_UPTODATE))
 		return;
 	bp->b_error = 0;
-	bp->b_ops = t->bops;
+	bp->b_ops = type->bops;
 	bp->b_ops->verify_read(bp);
 	bp->b_flags &= ~LIBXFS_B_UNCHECKED;
 }
diff --git a/db/io.h b/db/io.h
index b415b82d..99730048 100644
--- a/db/io.h
+++ b/db/io.h
@@ -59,10 +59,10 @@ extern void	print_iocur(char *tag, iocur_t *ioc);
 extern void	push_cur(void);
 extern int	read_buf(__int64_t daddr, int count, void *bufp);
 extern void     write_cur(void);
-extern void	set_cur(const struct typ *t, __int64_t d, int c, int ring_add,
-			bbmap_t *bbmap);
+extern void	set_cur(const struct typ *type, xfs_daddr_t blknum,
+			int len, int ring_add, bbmap_t *bbmap);
 extern void     ring_add(void);
-extern void	set_iocur_type(const struct typ *t);
+extern void	set_iocur_type(const struct typ *type);
 extern void	xfs_dummy_verify(struct xfs_buf *bp);
 extern void	xfs_verify_recalc_inode_crc(struct xfs_buf *bp);
 extern void	xfs_verify_recalc_dquot_crc(struct xfs_buf *bp);
-- 
2.13.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2017-06-28 19:26 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-28 15:09 [PATCH] xfs_db: improve argument naming in set_cur and set_iocur_type Bill O'Donnell
2017-06-28 16:40 ` Christoph Hellwig
2017-06-28 16:48 ` Darrick J. Wong
2017-06-28 18:04   ` Bill O'Donnell
2017-06-28 18:38 ` Eric Sandeen
2017-06-28 18:42   ` Bill O'Donnell
2017-06-28 19:25 ` [PATCH v2] " Bill O'Donnell

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.