#435 Compilation fix for 64 bits time data type on 32 bits architecture

[yokota]

Current Debian Linux trying to use 64 bits time data type even on 32 bits architecture.
This attempt breaks 7-Zip compilation on 32 bits architectures like this.

../../../Windows/TimeUtils.cpp: In function ‘void NWindows::NTime::GetCurUtc_FiTime(timespec&)’:
../../../Windows/TimeUtils.cpp:264:30: error: conversion from ‘__suseconds64_t’ {aka ‘long long int’} to ‘long int’ may change value [-Werror=conversion]
  264 |     ft.tv_nsec = now.tv_usec * 1000;
      |                  ~~~~~~~~~~~~^~~~~~

This patch fixes the error.
It uses std::timespec_get() to get current UTC time.
This function was available since C++17.

[Igor Pavlov]

So why timespec::tv_nsec is 32-bit and timeval::tv_usec is 64-bit?
Why that difference?

[yokota]

I don't know why, but timespec::tv_nsec and timeval::tv_usec uses different strategy to change data size to 64bits on 32bit architectures.
See the glibc repository and its commit history.

https://sourceware.org/git/?p=glibc.git;a=history;f=time/bits/types/struct_timespec.h;h=1141015f276cb33032a6d4ec70d5d73ec2c21d55;hb=HEAD
https://sourceware.org/git/?p=glibc.git;a=blob;f=time/bits/types/struct_timeval.h;h=0c8e88c82c400d686b0dccf99c62d1b16c16fc00;hb=HEAD

You can also check Debian's document about 64bit time transition.
https://wiki.debian.org/ReleaseGoals/64bit-time

Anyway, GetCurUtc_FiTime() just gets current UTC time in struct timespec.
I think std::timespec_get() is more simple method to get the time in struct timespec.

[Sam Tansy]

It's not very standard function. Not in time(7), nor in time(2) manual.

Also this 64-bit tv_usec, 44 bits overhead, is not in GCC-12 with its glibc that is 1 year younger than this perplexing change.

You have to turn off `-Werror' anyways to compile with GCC/Clang either way as it always throws some warnings, and compiler can do appropriate conversion without much fuss.

[yokota]

Not in time(7), nor in time(2) manual.

How about clock_gettime(2) ? It's POSIX function.
I was updated my patch. See the attached patch file below.

You have to turn off '-Werror'

-Werror was specified by upstream source code CPP/7zip/7zip_gcc.mak.

CFLAGS_WARN_WALL = -Werror -Wall -Wextra

Use -Werror -Wno-error=conversion instead of -Werror to disable error report for conversion issue.

CFLAGS_WARN_WALL = -Werror -Wno-error=conversion -Wall -Wextra

[Sam Tansy]

How about clock_gettime(2) ? It's POSIX function

It makes more sense, than including half libstdc++ for some peripheral function.

It will introduce additional dependency to glibc, or librt on older systems, and in mingw to lib(win)pthreads, because something used by real timer is in winpthreads. The last one can be solved by manual static linking of lib(win)pthreads.

I still don't get what the problem is. Why is this strange, inconsistent, change introduced to glibc, that (the changed version) no one, except some 'finge' debian use, such a problem, if compiler can do a conversion?
It depends what is GetCurUtc_FiTime() is for and how dev sees it.

-Werror was specified by upstream source code CPP/7zip/7zip_gcc.mak.

I know. I also never was able to compile it without a warning; no matter what Linux compiler I tried. And I tried many. So I always ended up removing `-Werror' to be able to continue.

[yokota]

I found why this issue happen.

It was comes from POSIX definition of timeval::tv_usec.

POSIX definition requires timespec::tv_nsec type is long.
But timeval::tv_usec type is suseconds_t and not determine exact data size.

long size may vary on 32/64 bits architecture.
It takes 32 bits for 32 bits architecture (ILP32), and 64 bits for 64 bits architecture (LP64).
FYI: MS Windows uses 32 bits for long and 64 bits for long long (LLP64).

suseconds_t size is not defined by POSIX, so it takes always 64 bits after 64 bits time migration.
And glibc developers adds "padding" hack around timeval::tv_usec to partially support 64 bits time on 32 bits environment. See commit history of struct_timespec.h above.

These differences makes value conversion warnings on 32 bits environment.

I was update my patch to use type cast for this issue.