Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 8 Aug 2019 10:46:29 -0500
From:      Justin Hibbits <jhibbits@FreeBSD.org>
To:        Ian Lepore <ian@freebsd.org>
Cc:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   Re: svn commit: r350737 - head/usr.sbin/autofs
Message-ID:  <20190808104629.405a5c0b@ralga.knownspace>
In-Reply-To: <53d227e7f8535424c3abff229a41e9375b077dcc.camel@freebsd.org>
References:  <201908080316.x783GW3r070041@repo.freebsd.org> <53d227e7f8535424c3abff229a41e9375b077dcc.camel@freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, 08 Aug 2019 08:00:16 -0600
Ian Lepore <ian@freebsd.org> wrote:

> On Thu, 2019-08-08 at 03:16 +0000, Justin Hibbits wrote:
> > Author: jhibbits
> > Date: Thu Aug  8 03:16:32 2019
> > New Revision: 350737
> > URL: https://svnweb.freebsd.org/changeset/base/350737
> > 
> > Log:
> >   Change autounmountd(8) to use time_t for duration instead of
> > double 
> >   Summary:
> >   autounmountd(8) uses doubles to handle mount time durations.
> > However, it must convert to integer types, time_t in particular, to
> > do anything meaningful.  Additionally, even though it's a
> > floating-point value in seconds, the sub-seconds component is never
> > used, so it's unnecessary. 
> >   Switching type to time_t fixes an assertion on powerpc64, which
> > checks that a sleep value that's not -1.0 is greater than 0.  On
> > powerpc64, it happens that the value of -1.0 gets loaded as a float
> > (perhaps a bug in gcc), but gets compared to a double.  This
> > compares as false, so follows through the 'sleep != -1.0' path, and
> > fails the assert.  Since the sub-second component isn't used in the
> > double, just drop it and deal with whole-integer seconds.
> >   
> >   Reviewed by:	trasz
> >   Differential Revision: https://reviews.freebsd.org/D21109
> > 
> > Modified:
> >   head/usr.sbin/autofs/autounmountd.c
> > 
> > Modified: head/usr.sbin/autofs/autounmountd.c
> > ==============================================================================
> > --- head/usr.sbin/autofs/autounmountd.c	Thu Aug  8 03:01:35
> > 2019	(r350736) +++
> > head/usr.sbin/autofs/autounmountd.c	Thu Aug  8 03:16:32
> > 2019	(r350737) @@ -179,12 +179,12 @@ unmount_by_fsid(const
> > fsid_t fsid, const char *mountpo return (error); }
> >  
> > -static double
> > -expire_automounted(double expiration_time)
> > +static time_t
> > +expire_automounted(time_t expiration_time)
> >  {
> >  	struct automounted_fs *af, *tmpaf;
> >  	time_t now;
> > -	double mounted_for, mounted_max = -1.0;
> > +	time_t mounted_for, mounted_max = -1;
> >  	int error;
> >  
> >  	now = time(NULL);
> > @@ -196,9 +196,9 @@ expire_automounted(double expiration_time)
> >  
> >  		if (mounted_for < expiration_time) {
> >  			log_debugx("skipping %s (FSID:%d:%d),
> > mounted "
> > -			    "for %.0f seconds", af->af_mountpoint,
> > +			    "for %ld  seconds", af->af_mountpoint,
> >  
> 
> You can't print a time_t with %ld, it'll fail on 32-bit arches with
> 64- bit time_t.  The usual incantation is %j and cast to intmax_t.
> 
> -- Ian
> 
> 

I cast it to a long already, so it prints as a long for all archs.  I
guess intmax_t might be better, but I figured nobody would hold a mount
for more than 2**31 seconds (the argument is for duration, not absolute
time).

- Justin



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20190808104629.405a5c0b>