Re: [PATCH v3 2/3] login: structs utmp, utmpx, lastlog _TIME_BITS independence (bug 30701)

[Florian Weimer <fweimer@redhat.com>]

* Paul Eggert:

> On 2024-04-09 23:45, Florian Weimer wrote:
>> diff --git a/sysdeps/x86/bits/wordsize.h b/sysdeps/x86/bits/wordsize.h
>> index 70f652bca1..3f40aa76f9 100644
>> --- a/sysdeps/x86/bits/wordsize.h
>> +++ b/sysdeps/x86/bits/wordsize.h
>> @@ -8,10 +8,9 @@
>>   #define __WORDSIZE32_PTRDIFF_LONG	0
>>   #endif
>>   +#define __WORDSIZE_TIME64_COMPAT32 1
>> +
>>   #ifdef __x86_64__
>> -# define __WORDSIZE_TIME64_COMPAT32	1
>>   /* Both x86-64 and x32 use the 64-bit system call interface.  */
>>   # define __SYSCALL_WORDSIZE		64
>> -#else
>> -# define __WORDSIZE_TIME64_COMPAT32	0
>>   #endif
>
> I'm still having a few qualms about this part of the patch, and
> similarly for ARM. To continue from an earlier round:
>
>>> > On 4/3/24 11:39, Florian Weimer wrote:
>>>> >> For consistency,
>>>> >> if there is a 64-bit architecture that is coinstallable, define
>>>> >> __WORDSIZE_TIME64_COMPAT32 to 1 on the 32-bit architectyre as well.
>>> >
>>> > Could you explain the advantage of consistency here? User code almost
>>> > invariably assignes ut_tv.tv_sec to time_t (this is true of every
>>> > instance I found of ut_tv in Gnulib source code, for example). So
>>> > changing this field's type on platforms where time_t is 32 bits will
>>> > likely be ineffective in practice, and might cause more problems than
>>> > it cures.
>> Few applications with a 32-bit time_t will work once there is a
>> value in
>> this field with the MSB set.  So the relevant case is applications that
>> were built with -D_TIME_BITS=64, and there the consistent behavior with
>> the 64-bit architecture helps.
>
> It appears that I was not clear, as I was worried about applications
> built on 32-bit ARM or x86 without -D_TIME_BITS=64, so please let me
> try again.
>
> For obsolescent 32-bit time_t apps that deal with 32-bit on-disk
> timestamps I see two forms of "consistent behavior":
>
> A. These obsolescent apps should behave consistently with how they've
> behaved for decades.
>
> B. These obsolescent apps should behave consistently with how 64-bit
> time_t apps work on the same platform for timestamps after 2038.
>
> Surely consistency (A) is more important than consistency (B).

Thank you, I think I better understood your position now.

The patch here is a source-only change.  Applications which are not
recompiled are unaffected by it.  So it does not necessarily contradict
(A), depending on how you read it.

With the present patch, the type follows this rule:

  If the field size is 64 bits, its type is time_t, otherwise the field
  type is uint32_t.

Your proposal would lead to a rule like this:

  If the field size is 64 bits, it is time_t.  If the field size is
  32 bits and time_t is 64 bits (in this translation unit), the field
  type is uint32_t.  Otherwise the field size is 32 bits and its type
  is int32_t.

From an application perspective, I don't think much changes.
Applications should process the field value as int64_t anyway, operating
on a copy.  (No conditional compilation should be needed.)  So no extra
conditional code should be needed compared to the simpler rule I
implemented.

The glibc header change would be a bit more involved because we
currently do not define __USE_TIME_BITS64 for Hurd, as far as I can see
(and the installed <features-time64.h> is probably garbled, so
<features.h> is broken?).  The tests probably would rely on a new macro
in <utmp-size.h>.  I can implement it that way, but I'm not sure if it's
worth the complexity.  This would only benefit obsolescent applications
that are recompiled.

Thanks,
Florian