diff mbox series

[05/26] xfs_quota: convert time_to_string to use time64_t

Message ID 160375527834.881414.2581158648212089750.stgit@magnolia (mailing list archive)
State Accepted
Headers show
Series xfsprogs: widen timestamps to deal with y2038 | expand

Commit Message

Darrick J. Wong Oct. 26, 2020, 11:34 p.m. UTC 
From: Darrick J. Wong <darrick.wong@oracle.com>

Rework the time_to_string helper to be capable of dealing with 64-bit
timestamps.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 quota/quota.c  |   16 ++++++++++------
 quota/quota.h  |    2 +-
 quota/report.c |   16 ++++++++++------
 quota/util.c   |    5 +++--
 4 files changed, 24 insertions(+), 15 deletions(-)

Comments

Christoph Hellwig Oct. 29, 2020, 9:47 a.m. UTC | #1 
On Mon, Oct 26, 2020 at 04:34:38PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Rework the time_to_string helper to be capable of dealing with 64-bit
> timestamps.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  quota/quota.c  |   16 ++++++++++------
>  quota/quota.h  |    2 +-
>  quota/report.c |   16 ++++++++++------
>  quota/util.c   |    5 +++--
>  4 files changed, 24 insertions(+), 15 deletions(-)
> 
> 
> diff --git a/quota/quota.c b/quota/quota.c
> index 9545cc430a93..8ba0995d9174 100644
> --- a/quota/quota.c
> +++ b/quota/quota.c
> @@ -48,6 +48,7 @@ quota_mount(
>  	uint		flags)
>  {
>  	fs_disk_quota_t	d;
> +	time64_t	timer;
>  	char		*dev = mount->fs_name;
>  	char		c[8], h[8], s[8];
>  	uint		qflags;
> @@ -100,6 +101,7 @@ quota_mount(
>  	}
>  
>  	if (form & XFS_BLOCK_QUOTA) {
> +		timer = d.d_btimer;
>  		qflags = (flags & HUMAN_FLAG);
>  		if (d.d_blk_hardlimit && d.d_bcount > d.d_blk_hardlimit)
>  			qflags |= LIMIT_FLAG;
> @@ -111,16 +113,17 @@ quota_mount(
>  				bbs_to_string(d.d_blk_softlimit, s, sizeof(s)),
>  				bbs_to_string(d.d_blk_hardlimit, h, sizeof(h)),
>  				d.d_bwarns,
> -				time_to_string(d.d_btimer, qflags));
> +				time_to_string(timer, qflags));

What do the local variables buy us over just relying on the implicit cast
to a larger integer type?
Darrick J. Wong Oct. 29, 2020, 5:32 p.m. UTC | #2 
On Thu, Oct 29, 2020 at 09:47:12AM +0000, Christoph Hellwig wrote:
> On Mon, Oct 26, 2020 at 04:34:38PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Rework the time_to_string helper to be capable of dealing with 64-bit
> > timestamps.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  quota/quota.c  |   16 ++++++++++------
> >  quota/quota.h  |    2 +-
> >  quota/report.c |   16 ++++++++++------
> >  quota/util.c   |    5 +++--
> >  4 files changed, 24 insertions(+), 15 deletions(-)
> > 
> > 
> > diff --git a/quota/quota.c b/quota/quota.c
> > index 9545cc430a93..8ba0995d9174 100644
> > --- a/quota/quota.c
> > +++ b/quota/quota.c
> > @@ -48,6 +48,7 @@ quota_mount(
> >  	uint		flags)
> >  {
> >  	fs_disk_quota_t	d;
> > +	time64_t	timer;
> >  	char		*dev = mount->fs_name;
> >  	char		c[8], h[8], s[8];
> >  	uint		qflags;
> > @@ -100,6 +101,7 @@ quota_mount(
> >  	}
> >  
> >  	if (form & XFS_BLOCK_QUOTA) {
> > +		timer = d.d_btimer;
> >  		qflags = (flags & HUMAN_FLAG);
> >  		if (d.d_blk_hardlimit && d.d_bcount > d.d_blk_hardlimit)
> >  			qflags |= LIMIT_FLAG;
> > @@ -111,16 +113,17 @@ quota_mount(
> >  				bbs_to_string(d.d_blk_softlimit, s, sizeof(s)),
> >  				bbs_to_string(d.d_blk_hardlimit, h, sizeof(h)),
> >  				d.d_bwarns,
> > -				time_to_string(d.d_btimer, qflags));
> > +				time_to_string(timer, qflags));
> 
> What do the local variables buy us over just relying on the implicit cast
> to a larger integer type?

It's a setup to avoid long lines of nested function call crud once we
get to patch 23.  Without the local variable, the fprintf turns into
this ugliness:

			fprintf(fp, " %6s %6s %6s  %02d %8s ",
				bbs_to_string(d.d_bcount, c, sizeof(c)),
				bbs_to_string(d.d_blk_softlimit, s, sizeof(s)),
				bbs_to_string(d.d_blk_hardlimit, h, sizeof(h)),
				d.d_bwarns,
				time_to_string(decode_timer(&d, d.d_itimer,
								d.d_itimer_hi),
						qflags));

Which I guess is also fine but I kind of hate function call inside
function call inside function call combined with high indent levels.

--D
Christoph Hellwig Oct. 30, 2020, 8:21 a.m. UTC | #3 
On Thu, Oct 29, 2020 at 10:32:34AM -0700, Darrick J. Wong wrote:
> It's a setup to avoid long lines of nested function call crud once we
> get to patch 23.  Without the local variable, the fprintf turns into
> this ugliness:

Ok.
Eric Sandeen Nov. 16, 2020, 8:45 p.m. UTC | #4 
On 10/26/20 6:34 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Rework the time_to_string helper to be capable of dealing with 64-bit
> timestamps.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

Looks ok to me, seems like hch's concern was resolved too.

Reviewed-by: Eric Sandeen <sandeen@redhat.com>
diff mbox series

Patch

diff --git a/quota/quota.c b/quota/quota.c
index 9545cc430a93..8ba0995d9174 100644
--- a/quota/quota.c
+++ b/quota/quota.c
@@ -48,6 +48,7 @@  quota_mount(
 	uint		flags)
 {
 	fs_disk_quota_t	d;
+	time64_t	timer;
 	char		*dev = mount->fs_name;
 	char		c[8], h[8], s[8];
 	uint		qflags;
@@ -100,6 +101,7 @@  quota_mount(
 	}
 
 	if (form & XFS_BLOCK_QUOTA) {
+		timer = d.d_btimer;
 		qflags = (flags & HUMAN_FLAG);
 		if (d.d_blk_hardlimit && d.d_bcount > d.d_blk_hardlimit)
 			qflags |= LIMIT_FLAG;
@@ -111,16 +113,17 @@  quota_mount(
 				bbs_to_string(d.d_blk_softlimit, s, sizeof(s)),
 				bbs_to_string(d.d_blk_hardlimit, h, sizeof(h)),
 				d.d_bwarns,
-				time_to_string(d.d_btimer, qflags));
+				time_to_string(timer, qflags));
 		else
 			fprintf(fp, " %10llu %10llu %10llu   %02d %9s ",
 				(unsigned long long)d.d_bcount >> 1,
 				(unsigned long long)d.d_blk_softlimit >> 1,
 				(unsigned long long)d.d_blk_hardlimit >> 1,
 				d.d_bwarns,
-				time_to_string(d.d_btimer, qflags));
+				time_to_string(timer, qflags));
 	}
 	if (form & XFS_INODE_QUOTA) {
+		timer = d.d_itimer;
 		qflags = (flags & HUMAN_FLAG);
 		if (d.d_ino_hardlimit && d.d_icount > d.d_ino_hardlimit)
 			qflags |= LIMIT_FLAG;
@@ -132,16 +135,17 @@  quota_mount(
 				num_to_string(d.d_ino_softlimit, s, sizeof(s)),
 				num_to_string(d.d_ino_hardlimit, h, sizeof(h)),
 				d.d_iwarns,
-				time_to_string(d.d_itimer, qflags));
+				time_to_string(timer, qflags));
 		else
 			fprintf(fp, " %10llu %10llu %10llu   %02d %9s ",
 				(unsigned long long)d.d_icount,
 				(unsigned long long)d.d_ino_softlimit,
 				(unsigned long long)d.d_ino_hardlimit,
 				d.d_iwarns,
-				time_to_string(d.d_itimer, qflags));
+				time_to_string(timer, qflags));
 	}
 	if (form & XFS_RTBLOCK_QUOTA) {
+		timer = d.d_rtbtimer;
 		qflags = (flags & HUMAN_FLAG);
 		if (d.d_rtb_hardlimit && d.d_rtbcount > d.d_rtb_hardlimit)
 			qflags |= LIMIT_FLAG;
@@ -153,14 +157,14 @@  quota_mount(
 				bbs_to_string(d.d_rtb_softlimit, s, sizeof(s)),
 				bbs_to_string(d.d_rtb_hardlimit, h, sizeof(h)),
 				d.d_rtbwarns,
-				time_to_string(d.d_rtbtimer, qflags));
+				time_to_string(timer, qflags));
 		else
 			fprintf(fp, " %10llu %10llu %10llu   %02d %9s ",
 				(unsigned long long)d.d_rtbcount >> 1,
 				(unsigned long long)d.d_rtb_softlimit >> 1,
 				(unsigned long long)d.d_rtb_hardlimit >> 1,
 				d.d_rtbwarns,
-				time_to_string(d.d_rtbtimer, qflags));
+				time_to_string(timer, qflags));
 	}
 	fprintf(fp, "%s\n", mount->fs_dir);
 	return 1;
diff --git a/quota/quota.h b/quota/quota.h
index 025d887726d8..11f62b208e6a 100644
--- a/quota/quota.h
+++ b/quota/quota.h
@@ -40,7 +40,7 @@  enum {
  */
 extern char *type_to_string(uint __type);
 extern char *form_to_string(uint __form);
-extern char *time_to_string(time_t __time, uint __flags);
+extern char *time_to_string(time64_t __time, uint __flags);
 extern char *bbs_to_string(uint64_t __v, char *__c, uint __size);
 extern char *num_to_string(uint64_t __v, char *__c, uint __size);
 extern char *pct_to_string(uint64_t __v, uint64_t __t, char *__c, uint __s);
diff --git a/quota/report.c b/quota/report.c
index e6def916b827..2d5024e95177 100644
--- a/quota/report.c
+++ b/quota/report.c
@@ -330,6 +330,7 @@  report_mount(
 	uint		flags)
 {
 	fs_disk_quota_t	d;
+	time64_t	timer;
 	char		*dev = mount->fs_name;
 	char		c[8], h[8], s[8];
 	uint		qflags;
@@ -397,6 +398,7 @@  report_mount(
 	}
 
 	if (form & XFS_BLOCK_QUOTA) {
+		timer = d.d_btimer;
 		qflags = (flags & HUMAN_FLAG);
 		if (d.d_blk_hardlimit && d.d_bcount > d.d_blk_hardlimit)
 			qflags |= LIMIT_FLAG;
@@ -408,16 +410,17 @@  report_mount(
 				bbs_to_string(d.d_blk_softlimit, s, sizeof(s)),
 				bbs_to_string(d.d_blk_hardlimit, h, sizeof(h)),
 				d.d_bwarns,
-				time_to_string(d.d_btimer, qflags));
+				time_to_string(timer, qflags));
 		else
 			fprintf(fp, " %10llu %10llu %10llu     %02d %9s",
 				(unsigned long long)d.d_bcount >> 1,
 				(unsigned long long)d.d_blk_softlimit >> 1,
 				(unsigned long long)d.d_blk_hardlimit >> 1,
 				d.d_bwarns,
-				time_to_string(d.d_btimer, qflags));
+				time_to_string(timer, qflags));
 	}
 	if (form & XFS_INODE_QUOTA) {
+		timer = d.d_itimer;
 		qflags = (flags & HUMAN_FLAG);
 		if (d.d_ino_hardlimit && d.d_icount > d.d_ino_hardlimit)
 			qflags |= LIMIT_FLAG;
@@ -429,16 +432,17 @@  report_mount(
 				num_to_string(d.d_ino_softlimit, s, sizeof(s)),
 				num_to_string(d.d_ino_hardlimit, h, sizeof(h)),
 				d.d_iwarns,
-				time_to_string(d.d_itimer, qflags));
+				time_to_string(timer, qflags));
 		else
 			fprintf(fp, " %10llu %10llu %10llu     %02d %9s",
 				(unsigned long long)d.d_icount,
 				(unsigned long long)d.d_ino_softlimit,
 				(unsigned long long)d.d_ino_hardlimit,
 				d.d_iwarns,
-				time_to_string(d.d_itimer, qflags));
+				time_to_string(timer, qflags));
 	}
 	if (form & XFS_RTBLOCK_QUOTA) {
+		timer = d.d_rtbtimer;
 		qflags = (flags & HUMAN_FLAG);
 		if (d.d_rtb_hardlimit && d.d_rtbcount > d.d_rtb_hardlimit)
 			qflags |= LIMIT_FLAG;
@@ -450,14 +454,14 @@  report_mount(
 				bbs_to_string(d.d_rtb_softlimit, s, sizeof(s)),
 				bbs_to_string(d.d_rtb_hardlimit, h, sizeof(h)),
 				d.d_rtbwarns,
-				time_to_string(d.d_rtbtimer, qflags));
+				time_to_string(timer, qflags));
 		else
 			fprintf(fp, " %10llu %10llu %10llu     %02d %9s",
 				(unsigned long long)d.d_rtbcount >> 1,
 				(unsigned long long)d.d_rtb_softlimit >> 1,
 				(unsigned long long)d.d_rtb_hardlimit >> 1,
 				d.d_rtbwarns,
-				time_to_string(d.d_rtbtimer, qflags));
+				time_to_string(timer, qflags));
 	}
 	fputc('\n', fp);
 	return 1;
diff --git a/quota/util.c b/quota/util.c
index 50470aba7b05..361d2a8ef5c6 100644
--- a/quota/util.c
+++ b/quota/util.c
@@ -18,11 +18,12 @@ 
 
 char *
 time_to_string(
-	time_t		origin,
+	time64_t	origin,
 	uint		flags)
 {
 	static char	timestamp[32];
-	time_t		now, timer;
+	time64_t	timer;
+	time_t		now;
 	uint		days, hours, minutes, seconds;
 
 	if (flags & ABSOLUTE_FLAG) {