https_dns_proxy: logging fix

[stangri]

This fix is based on @schmaller's patch and @systemcrash testing from this issue: openwrt/packages#19366

Credit goes to @schmaller and @systemcrash.

[systemcrash]

This fixes compiler errors, as well as legitimate crashes:

/home/user/openwrt/build_dir/target-mips_24kc_musl/https-dns-proxy-2023-05-25/src/logging.c: In function '_log':
/home/user/openwrt/build_dir/target-mips_24kc_musl/https-dns-proxy-2023-05-25/src/logging.c:81:24: warning: format '%lu' expects argument of type 'long unsigned int', but argument 4 has type 'long long unsigned int' [-Wformat=]
   81 |   fprintf(logf, "%s %8lu.%06lu %s:%d ", SeverityStr[severity],
      |                     ~~~^
      |                        |
      |                        long unsigned int
      |                     %8llu
   82 |           (uint64_t)tv.tv_sec,
      |           ~~~~~~~~~~~~~~~~~~~
      |           |
      |           long long unsigned int
/home/user/openwrt/build_dir/target-mips_24kc_musl/https-dns-proxy-2023-05-25/src/logging.c:81:30: warning: format '%lu' expects argument of type 'long unsigned int', but argument 5 has type 'long long unsigned int' [-Wformat=]
   81 |   fprintf(logf, "%s %8lu.%06lu %s:%d ", SeverityStr[severity],
      |                          ~~~~^
      |                              |
      |                              long unsigned int
      |                          %06llu
   82 |           (uint64_t)tv.tv_sec,
   83 |           (uint64_t)tv.tv_usec, file, line);
      |           ~~~~~~~~~~~~~~~~~~~~
      |           |
      |           long long unsigned int

[baranyaib90]

I agree, that this fix is needed.

[baranyaib90]

Actually this does not work well on Ubuntu 22.04 64bit :S
I would go into the PRIu64 direction.

https_dns_proxy/src/logging.c:82:11: warning: format specifies type 'unsigned long long' but the argument has type 'uint64_t' (aka 'unsigned long') [clang-diagnostic-format]
          (uint64_t)tv.tv_sec,
          ^~~~~~~~~~~~~~~~~~~
https_dns_proxy/src/logging.c:81:27: note: FIX-IT applied suggested code changes
  (void)fprintf(logf, "%s %8llu.%06llu %s:%d ", SeverityStr[severity],
                          ^
https_dns_proxy/src/logging.c:83:11: warning: format specifies type 'unsigned long long' but the argument has type 'uint64_t' (aka 'unsigned long') [clang-diagnostic-format]
          (uint64_t)tv.tv_usec, file, line);
          ^~~~~~~~~~~~~~~~~~~~
https_dns_proxy/src/logging.c:81:33: note: FIX-IT applied suggested code changes
  (void)fprintf(logf, "%s %8llu.%06llu %s:%d ", SeverityStr[severity],
                                ^

Ps.: even clang-tidy shows the warning at Build step
https://github.com/aarond10/https_dns_proxy/actions/runs/6528150677/job/17723933346?pr=161

[stangri]

@baranyaib90 I've already merged the patch to OpenWrt packages repo, so it's fixed there.

[baranyaib90]

Patch might fix OpenWRT in your case. I'm unsure, that it actually works for all target (32/64 bit and plenty architectures).
Anyways according to clang-tidy it's incorrect :S Time will tell which works best.

[systemcrash]

Well, here's the rub: time_t is 32 bit on 32 bit systems+compilers. 64 bit on 64 bit systems.

https://lwn.net/Articles/598408/
https://www.gnu.org/software/libc/manual/html_node/64_002dbit-time-symbol-handling.html

In any case, if it's only this one fprintf then an #ifdef might suffice. What is certain, is that a 64 bit value will be needed in the future.

unsigned long is 32 bits and unsigned long long is equivalent to uint64. The assertion 'uint64_t' (aka 'unsigned long') feels wrong unless the compiler OS is 32bit...

[baranyaib90]

My solution just got merged on master: bd71243
I would recommend to try that one out. I'm using PRIu64 format specifier for the uint64_t type. (So casting time_t to uint64_t is necessary, just the printf format specifier cause trouble.)
My point is, that inside inttypes.h it's handled via define if the code is being compiled on 32 or 64 bit target.
So PRIu64 value is "llu" on 32 bit and "lu" on 64 bit.

I recommend to close this pull request (since this patch causes trouble on 64 bit) and use my hopefully more generic fix.

[systemcrash]

So PRIu64 value is "llu" on 32 bit and "lu" on 64 bit.

You probably meant vice-versa. But this seems like a promising fix. It just requires testing in situ on our platforms.

[baranyaib90]

I'm pretty sure I didn't swap them. On 32 bit more "l" letters are needed.
https://github.com/lattera/glibc/blob/master/sysdeps/generic/inttypes.h#L43
https://git.musl-libc.org/cgit/musl/tree/include/inttypes.h#n25

[stangri]

@systemcrash if you want to test the new code, I've updated makefile and patches in stangri/source.openwrt.melmac.net@2358e71.

[stangri]

I feel it's OK to close this, I'd like to confirm the @baranyaib90's patch works tho.

[systemcrash]

Looks good on my side. Recompiled and installed on my ath79. No crash.