Re: [RFC PATCH v5 0/8] rebase-interactive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 3/26/2018 11:56 AM, Junio C Hamano wrote:
Wink Saville <wink@xxxxxxxxxxx> writes:

json-writer.c:123:38:  error:  format specifies type 'uintmax_t' (aka
'unsigned long') but the argument has type 'uint64_t' (aka 'unsigned
long long') [-Werror,-Wformat]

         strbuf_addf(&jw->json, ":%"PRIuMAX, value);
                                  ~~         ^~~~~
json-writer.c:228:37:  error:  format specifies type 'uintmax_t' (aka
'unsigned long') but the argument has type 'uint64_t' (aka 'unsigned
long long') [-Werror,-Wformat] [0m

         strbuf_addf(&jw->json, "%"PRIuMAX, value);
                                  ~~         ^~~~~
2 errors generated.
make: *** [json-writer.o] Error 1
make: *** Waiting for unfinished jobs....

For whatever reason, our codebase seems to shy away from PRIu64,
even though there are liberal uses of PRIu32.  Showing the value
casted to uintmax_t with PRIuMAX seems to be our preferred way to
say "We cannot say how wide this type is on different platforms, and
are playing safe by using widest-possible int type" (e.g. showing a
pid_t value from daemon.c).

In this codepath, the actual values are specified to be uint64_t, so
the use of PRIu64 may be OK, but I have to wonder why the codepath
is not dealing with uintmax_t in the first place.  When even larger
than present archs are prevalent in N years and 64-bit starts to
feel a tad small (like we feel for 16-bit ints these days), it will
feel a bit silly to have a subsystem that is limited to such a
"fixed and a tad small these days" types and pretend it to be be a
generic seriealizer, I suspect.


I defined that routine to take a uint64_t because I wanted to
pass a nanosecond value received from getnanotime() and that's
what it returns.

My preference would be to change the PRIuMAX to PRIu64, but there
aren't any other references in the code to that symbol and I didn't
want to start a new trend here.

I am concerned that the above compiler error message says that uintmax_t
is defined as an "unsigned long" (which is defined as *at least* 32 bits,
but not necessarily 64.  But a uint64_t is defined as a "unsigned long long"
and guaranteed as a 64 bit value.

So while I'm not really worried about 128 bit integers right now, I'm
more concerned about 32 bit compilers truncating that value without any
warnings.

Jeff




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux