[PATCH] debuginfod/debuginfod-client.c: correct string format on 32bit arches with 64bit time_t

Khem Raj raj.khem@gmail.com
Sat Nov 20 14:41:41 GMT 2021

On Sat, Nov 20, 2021 at 12:11 AM Alexander Kanavin
<alex.kanavin@gmail.com> wrote:
> On Sat, 20 Nov 2021 at 05:13, Érico Nogueira <ericonr@disroot.org> wrote:
>> For what it's worth, most of the time64 support patches that I have seen
>> use "%lld" and `long long` as the type for portable representation of
>> time, instead of intmax_t, but each should work just as well as the
>> other.
> My original version did use %lld, but Khem tweaked it to use intmax_t. Perhaps he can comment?

intmax_t is not particularly bounded like long long int and sounded
the right choice here say if we have 128bit
machines in future.

>> Might be worth mentioning in the commit that musl 1.2.0 and above uses
>> time64 on all platforms, and that glibc 2.34 has added support for
>> time64 (which might be enabled by distro-wide CFLAGS), with possibly
>> 2.35 or 2.36 making it enabled by default.
> Yes, I can add that.
>> This is the wrong fix, a temporary variable should be used. When time_t
>> is still 32 bits, it means you can accidentally write out of bounds for
>> times after 2038 on little endian plaforms; and on big endian platforms,
>> it's always writing out of bounds (and *only out of bounds*, before
>> 2038, so you can't even access the result of fscanf).

yes it seems that way, thanks for catching it.`

>> Having now taken a second look at the code, I don't think this needs to
>> be treated as time_t, anyway? cache_config is a `long`, and it's reading
>> a max timeout value, which is unlikely to go beyond the hundreds of
>> seconds... Given that the function returns an `int`, I'd even consider
>> making `cache_config` an `int` directly.
> I can fix this as well if Khem confirms.

treating int is fine. debuginfod_config_cache() is still passed time_t as second
param in few call sites which are assigned to cache_config here, so we will
still need to convert/truncate that assignment.

> Thanks for the review.
> Alex

More information about the Elfutils-devel mailing list