[PATCH] xfs_quota: check report_mount return value

[PATCH] xfs_quota: check report_mount return value

[Eric Sandeen]

The new call to report_mount doesn't check the return value
like every other caller does...

Returning 1 means it printed something; if the terse flag
is used and there is no usage, nothing gets printed.
If we set the NO_HEADER_FLAG anyway, then we won't see
the header for subsequent entries as we expect.

For example, project ID 0 has no usage in this case:

# xfs_quota -x -c "report -a" /mnt/test
Project quota on /mnt/test (/dev/sdb1)
                               Blocks                     
Project ID       Used       Soft       Hard    Warn/Grace     
---------- -------------------------------------------------- 
#0                  0          0          0     00 [--------]
project          2048          4          4     00 [--none--]

So using the terse flag results in no header when it prints
projects with usage:

# xfs_quota -x -c "report -t -a" /mnt/test
project          2048          4          4     00 [--none--]

With this fix it prints the header as expected:

# xfs_quota -x -c "report -t -a" /mnt/test
Project quota on /mnt/test (/dev/sdb1)
                               Blocks                     
Project ID       Used       Soft       Hard    Warn/Grace     
---------- -------------------------------------------------- 
project          2048          4          4     00 [--none--]


Addresses-Coverity-Id: 1361552
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

diff --git a/quota/report.c b/quota/report.c
index cc422d1..59290e1 100644
--- a/quota/report.c
+++ b/quota/report.c
@@ -580,9 +580,9 @@ report_project_mount(
 			 * Print default project quota, even if projid 0
 			 * isn't defined
 			 */
-			report_mount(fp, 0, NULL, NULL, form, XFS_PROJ_QUOTA,
-			             mount, flags);
-			flags |= NO_HEADER_FLAG;
+			if (report_mount(fp, 0, NULL, NULL,
+					form, XFS_PROJ_QUOTA, mount, flags))
+				flags |= NO_HEADER_FLAG;
 		}
 
 		setprent();

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfs_quota: check report_mount return value

[Zorro Lang]

On Wed, May 11, 2016 at 09:41:12PM -0500, Eric Sandeen wrote:
> The new call to report_mount doesn't check the return value
> like every other caller does...
> 
> Returning 1 means it printed something; if the terse flag
> is used and there is no usage, nothing gets printed.
> If we set the NO_HEADER_FLAG anyway, then we won't see
> the header for subsequent entries as we expect.
> 
> For example, project ID 0 has no usage in this case:
> 
> # xfs_quota -x -c "report -a" /mnt/test
> Project quota on /mnt/test (/dev/sdb1)
>                                Blocks                     
> Project ID       Used       Soft       Hard    Warn/Grace     
> ---------- -------------------------------------------------- 
> #0                  0          0          0     00 [--------]
> project          2048          4          4     00 [--none--]
> 
> So using the terse flag results in no header when it prints
> projects with usage:
> 
> # xfs_quota -x -c "report -t -a" /mnt/test
> project          2048          4          4     00 [--none--]
> 
> With this fix it prints the header as expected:
> 
> # xfs_quota -x -c "report -t -a" /mnt/test
> Project quota on /mnt/test (/dev/sdb1)
>                                Blocks                     
> Project ID       Used       Soft       Hard    Warn/Grace     
> ---------- -------------------------------------------------- 
> project          2048          4          4     00 [--none--]
> 
> 
> Addresses-Coverity-Id: 1361552
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> 
> diff --git a/quota/report.c b/quota/report.c
> index cc422d1..59290e1 100644
> --- a/quota/report.c
> +++ b/quota/report.c
> @@ -580,9 +580,9 @@ report_project_mount(
>  			 * Print default project quota, even if projid 0
>  			 * isn't defined
>  			 */
> -			report_mount(fp, 0, NULL, NULL, form, XFS_PROJ_QUOTA,
> -			             mount, flags);
> -			flags |= NO_HEADER_FLAG;
> +			if (report_mount(fp, 0, NULL, NULL,
> +					form, XFS_PROJ_QUOTA, mount, flags))
> +				flags |= NO_HEADER_FLAG;

Wow, you're right! TERSE MODE will return 0 if no any quota
limit:
        if (flags & TERSE_FLAG) {
                count = 0;
                if ((form & XFS_BLOCK_QUOTA) && d.d_bcount)
                        count++;
                if ((form & XFS_INODE_QUOTA) && d.d_icount)
                        count++;
                if ((form & XFS_RTBLOCK_QUOTA) && d.d_rtbcount)
                        count++;
                if (!count)
                        return 0;
        }

Thanks for fix my mistake. I can add TERSE report functional
test into my xfs/106 patch(which you're reviewing). Then it
can find bugs likes this ASAP.

Thanks,
Zorro

>  		}
>  
>  		setprent();
> 

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfs_quota: check report_mount return value

[Zorro Lang]

On Thu, May 12, 2016 at 11:15:00AM +0800, Zorro Lang wrote:
> On Wed, May 11, 2016 at 09:41:12PM -0500, Eric Sandeen wrote:
> > The new call to report_mount doesn't check the return value
> > like every other caller does...
> > 
> > Returning 1 means it printed something; if the terse flag
> > is used and there is no usage, nothing gets printed.
> > If we set the NO_HEADER_FLAG anyway, then we won't see
> > the header for subsequent entries as we expect.
> > 
> > For example, project ID 0 has no usage in this case:
> > 
> > # xfs_quota -x -c "report -a" /mnt/test
> > Project quota on /mnt/test (/dev/sdb1)
> >                                Blocks                     
> > Project ID       Used       Soft       Hard    Warn/Grace     
> > ---------- -------------------------------------------------- 
> > #0                  0          0          0     00 [--------]
> > project          2048          4          4     00 [--none--]
> > 
> > So using the terse flag results in no header when it prints
> > projects with usage:
> > 
> > # xfs_quota -x -c "report -t -a" /mnt/test
> > project          2048          4          4     00 [--none--]
> > 
> > With this fix it prints the header as expected:
> > 
> > # xfs_quota -x -c "report -t -a" /mnt/test
> > Project quota on /mnt/test (/dev/sdb1)
> >                                Blocks                     
> > Project ID       Used       Soft       Hard    Warn/Grace     
> > ---------- -------------------------------------------------- 
> > project          2048          4          4     00 [--none--]
> > 
> > 
> > Addresses-Coverity-Id: 1361552
> > Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> > ---
> > 
> > diff --git a/quota/report.c b/quota/report.c
> > index cc422d1..59290e1 100644
> > --- a/quota/report.c
> > +++ b/quota/report.c
> > @@ -580,9 +580,9 @@ report_project_mount(
> >  			 * Print default project quota, even if projid 0
> >  			 * isn't defined
> >  			 */
> > -			report_mount(fp, 0, NULL, NULL, form, XFS_PROJ_QUOTA,
> > -			             mount, flags);
> > -			flags |= NO_HEADER_FLAG;
> > +			if (report_mount(fp, 0, NULL, NULL,
> > +					form, XFS_PROJ_QUOTA, mount, flags))
> > +				flags |= NO_HEADER_FLAG;

Reviewed-by: Zorro Lang <zlang@redhat.com>

> 
> Wow, you're right! TERSE MODE will return 0 if no any quota
> limit:
>         if (flags & TERSE_FLAG) {
>                 count = 0;
>                 if ((form & XFS_BLOCK_QUOTA) && d.d_bcount)
>                         count++;
>                 if ((form & XFS_INODE_QUOTA) && d.d_icount)
>                         count++;
>                 if ((form & XFS_RTBLOCK_QUOTA) && d.d_rtbcount)
>                         count++;
>                 if (!count)
>                         return 0;
>         }
> 
> Thanks for fix my mistake. I can add TERSE report functional
> test into my xfs/106 patch(which you're reviewing). Then it
> can find bugs likes this ASAP.
> 
> Thanks,
> Zorro
> 
> >  		}
> >  
> >  		setprent();
> > 
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfs_quota: check report_mount return value

[Christoph Hellwig]

Looks good,

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

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfs_quota: check report_mount return value

[Christoph Hellwig]

On Thu, May 12, 2016 at 11:15:00AM +0800, Zorro Lang wrote:
> Thanks for fix my mistake. I can add TERSE report functional
> test into my xfs/106 patch(which you're reviewing). Then it
> can find bugs likes this ASAP.

Yes, please.  Additional test coverage is always good.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs