Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH] nfsdcltrack: Use uint64_t instead of time_t
To:     Jeff Layton <jlayton@kernel.org>,
        Linux NFS Mailing list <linux-nfs@vger.kernel.org>
References: <20210728013608.167759-1-steved@redhat.com>
 <0ba5eaacd17a50b0ab0c6fd9605a7c330935eca8.camel@kernel.org>
From:   Steve Dickson <steved@redhat.com>
Message-ID: <eed93f64-3394-5166-8d51-3075e866c327@redhat.com>
Date:   Wed, 28 Jul 2021 12:46:53 -0400
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.11.0
MIME-Version: 1.0
In-Reply-To: <0ba5eaacd17a50b0ab0c6fd9605a7c330935eca8.camel@kernel.org>
Content-Type: text/plain; charset=iso-8859-15; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 7/28/21 7:15 AM, Jeff Layton wrote:
> On Tue, 2021-07-27 at 21:36 -0400, Steve Dickson wrote:
>> With recent commits (4f2a5b64,5a53426c) that fixed
>> compile errors on x86_64 machines, caused similar
>> errors on i686 machines.
>>
>> The variable type that was being used was a time_t,
>> which changes size between architects, which
>> caused the compile error.
>>
>> Changing the variable to uint64_t fixed the issue.
>>
>> Signed-off-by: Steve Dickson <steved@redhat.com>
>> ---
>>   utils/nfsdcltrack/nfsdcltrack.c | 2 +-
>>   utils/nfsdcltrack/sqlite.c      | 2 +-
>>   utils/nfsdcltrack/sqlite.h      | 2 +-
>>   3 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/utils/nfsdcltrack/nfsdcltrack.c b/utils/nfsdcltrack/nfsdcltrack.c
>> index 0b37c094..7c1c4bcc 100644
>> --- a/utils/nfsdcltrack/nfsdcltrack.c
>> +++ b/utils/nfsdcltrack/nfsdcltrack.c
>> @@ -508,7 +508,7 @@ cltrack_gracedone(const char *timestr)
>>   {
>>   	int ret;
>>   	char *tail;
>> -	time_t gracetime;
>> +	uint64_t gracetime;
>>   
> 
> Hmm.. time_t is a long:
> 
>      typedef __kernel_long_t __kernel_time_t;
> 
> ...but the kernel is converting this value from a time64_t which is s64.
>   
> Should the above be int64_t instead of being unsigned? The kernel should
> never send down a negative value, but if you're trying to match up types
> then that might be cleaner.
The patch I took to fix the printfs on  64-bits platforms
used the PRIu64 inttype interface so I was just keeping
things constant by using a uint64_t.

The answer to your question is yes... int64 is all that
is needed... but I don't in really matters...

steved.
> 
>>
>>   	ret = sqlite_prepare_dbh(storagedir);
>> diff --git a/utils/nfsdcltrack/sqlite.c b/utils/nfsdcltrack/sqlite.c
>> index cea4a411..cf0c6a45 100644
>> --- a/utils/nfsdcltrack/sqlite.c
>> +++ b/utils/nfsdcltrack/sqlite.c
>> @@ -540,7 +540,7 @@ out_err:
>>    * remove any client records that were not reclaimed since grace_start.
>>    */
>>   int
>> -sqlite_remove_unreclaimed(time_t grace_start)
>> +sqlite_remove_unreclaimed(uint64_t grace_start)
>>   {
>>   	int ret;
>>   	char *err = NULL;
>> diff --git a/utils/nfsdcltrack/sqlite.h b/utils/nfsdcltrack/sqlite.h
>> index 06e7c044..ba8cdfa8 100644
>> --- a/utils/nfsdcltrack/sqlite.h
>> +++ b/utils/nfsdcltrack/sqlite.h
>> @@ -26,7 +26,7 @@ int sqlite_insert_client(const unsigned char *clname, const size_t namelen,
>>   int sqlite_remove_client(const unsigned char *clname, const size_t namelen);
>>   int sqlite_check_client(const unsigned char *clname, const size_t namelen,
>>   				const bool has_session);
>> -int sqlite_remove_unreclaimed(const time_t grace_start);
>> +int sqlite_remove_unreclaimed(const uint64_t grace_start);
>>   int sqlite_query_reclaiming(const time_t grace_start);
>>   
>>   #endif /* _SQLITE_H */
>