From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-vk1-xa29.google.com (mail-vk1-xa29.google.com [IPv6:2607:f8b0:4864:20::a29]) by sourceware.org (Postfix) with ESMTPS id 756793858425 for ; Sat, 20 Nov 2021 08:11:53 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 756793858425 Received: by mail-vk1-xa29.google.com with SMTP id 188so4255530vku.8 for ; Sat, 20 Nov 2021 00:11:53 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=8IhPOR48neCm6TpN06wsTjbasV2ACHX6xBHKKTL7YZI=; b=k8TdKkGrt5C3EY63/E7VU0LWCW6kB/fCbUqKceb3/UEVojH+tM/LVIdjkMuGq85aOO NlUSqXJutd4FkVz0yrpzq/20FtebKH76mttSDz1hI8fToOc5MVjMy2/pKVDRmblRFsZP mWzhGP0S5H22RpeNI9rI7DR4z146Yy1F3uUnSmcScRrNMwjcJ+YqirWci0sBHiJ08A+e XCZUWB9cAMA700FMnS1jR/qzPn8yt06B+rkFHYuJtkEYfxEPL32qB05v5ha7JqBAJrFZ 9GLKDcdv3n1NZhi5FWzV7AcvpGuwUq0yzyvsaeMwFt2ZWBbQ3L9ti4KuPgB210f95lRc eohg== X-Gm-Message-State: AOAM532a9mZVl0YY70uEyhVBF8zyMSraZ1gi1KIykcgeoYDQKIUhTL3o Uja6faSUtT2q6Nr+YIIGzUtYx5QRF5qm/qYmewQ= X-Google-Smtp-Source: ABdhPJwGaThv8oWt4IL4sjs5c9WT2mwpmO5pyWvHYKhhNkD47Cq5BGWSBb70Zzoe6kwl123L7GtUzsb9YeEByF1sM5I= X-Received: by 2002:a05:6122:a03:: with SMTP id 3mr129922901vkn.8.1637395913145; Sat, 20 Nov 2021 00:11:53 -0800 (PST) MIME-Version: 1.0 References: <20211119181524.3190417-1-alex@linutronix.de> In-Reply-To: From: Alexander Kanavin Date: Sat, 20 Nov 2021 09:11:41 +0100 Message-ID: Subject: Re: [PATCH] debuginfod/debuginfod-client.c: correct string format on 32bit arches with 64bit time_t To: =?UTF-8?Q?=C3=89rico_Nogueira?= Cc: Mark Wielaard , elfutils-devel@sourceware.org, Khem Raj X-Spam-Status: No, score=-3.1 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, HTML_MESSAGE, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Content-Filtered-By: Mailman/MimeDel 2.1.29 X-BeenThere: elfutils-devel@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Elfutils-devel mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 20 Nov 2021 08:11:54 -0000 On Sat, 20 Nov 2021 at 05:13, =C3=89rico Nogueira wro= te: > 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? > 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). > > 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. Thanks for the review. Alex