Re: [Y2038] [PATCH] misc: ibmasm: Replace timeval with timespec64

2015-10-23 Thread Arnd Bergmann
On Friday 23 October 2015 08:55:22 Amitoj Kaur Chawla wrote:
> >
> > Just to clarify, I should change this to
> >sprintf(buf, "%llu.%8.8lu", now.tv_sec, now.tv_nsec / NSEC_PER_USEC);
> >
> 
> Sorry, it should be changed to
> sprintf(buf, "%llu.%.08lu", (long long)now.tv_sec, now.tv_nsec /
> NSEC_PER_USEC);
> 

Yes, the last version of that looks correct to me.

Arnd
___
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038


Re: [Y2038] [PATCH] misc: ibmasm: Replace timeval with timespec64

2015-10-22 Thread Amitoj Kaur Chawla
On Fri, Oct 23, 2015 at 12:45 AM, Arnd Bergmann  wrote:
> On Thursday 22 October 2015 23:09:20 Amitoj Kaur Chawla wrote:
>> This patch replaces timeval with timespec64 as 32 bit 'struct timeval'
>> will not give current time beyond 2038.
>>
>> This also changes the code to use ktime_get_real_ts64() which returns
>> a 'struct timespec64' instead of do_gettimeofday() which returns a
>> 'struct timeval'
>>
>> Signed-off-by: Amitoj Kaur Chawla 
>
> Looks almost right, except for one bit:
>
>> ---
>>  drivers/misc/ibmasm/ibmasm.h | 7 ---
>>  1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/misc/ibmasm/ibmasm.h b/drivers/misc/ibmasm/ibmasm.h
>> index 9b08344..0c7bb6d 100644
>> --- a/drivers/misc/ibmasm/ibmasm.h
>> +++ b/drivers/misc/ibmasm/ibmasm.h
>> @@ -34,6 +34,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>
>>  /* Driver identification */
>>  #define DRIVER_NAME"ibmasm"
>> @@ -53,9 +54,9 @@ extern int ibmasm_debug;
>>
>>  static inline char *get_timestamp(char *buf)
>>  {
>> -   struct timeval now;
>> -   do_gettimeofday();
>> -   sprintf(buf, "%lu.%lu", now.tv_sec, now.tv_usec);
>> +   struct timespec64 now;
>> +   ktime_get_real_ts64();
>> +   sprintf(buf, "%lu.%lu", now.tv_sec, now.tv_nsec / NSEC_PER_USEC);
>> return buf;
>>
>
> now.tv_sec is using a 'time_t' a.k.a. 'long' on 64-bit architectures, but
> a 'time64_t' a.k.a. 'long long' on 32-bit architectures. This is a bit
> nonintuitive, but please change the format string to %llu for the first
> argument and add a corresponding cast to 's64' or 'long long', so we
> actually print the correct data and do not get a compiler warning.
>
> The tv_nsec argument is always 'long', on both 32-bit and 64-bit
> architectures, so there is no warning and we correctly print the
> number. However, there is a preexisting bug in the format string
> that you should fix while you are here: %lu drops leading zeroes,
> which results in something like "12345678.1234" where we want to
> see "1234567.001234". Please change the format string to add the
> correct number of leading zeroes here.
>

Just to clarify, I should change this to
   sprintf(buf, "%llu.%8.8lu", now.tv_sec, now.tv_nsec / NSEC_PER_USEC);

> As you seem to generally be doing well with the trivial patches, please
> move on to the 'simple' category next. I should also add a few more
> 'medium' level tasks but need to pick them carefully first.
>

Okay! Next patches will be the simple tasks. Also, I couldn't find the
tutorial you mentioned for compiling, can you help me out some there?

Thanks,
-- 
Amitoj
___
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038