Source-Changes-D archive

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

Re: CVS commit: src/usr.bin/make



On 26.01.2021 11:19, Rin Okuyama wrote:
Ping?

I don't think this is correct fix either.
Can you please revert this commit?

Sorry, I didn't get the first mail from Christos back in December,
that's why I didn't take any action.

Why shouldn't the fix I did be correct?  I carefully checked the few
places where I added the casts, and none of the numbers that are
involved will ever be more than a billion.

1.  The number of variables in a .for loop

2.  The number of items in a .for loop

3.  A single line of a text file in meta mode.  (Only in debug mode.)

4.  The number of lines in a single makefile.

5.  The capturing subexpression in a :C variable modifier, which only
ever ranges from 0 to 9.

6.  The number of words into which a variable value is split. (Only in
debug mode.)

Given these, why should a simple %u not be appropriate?  I don't want to
clutter the code with another preprocessor macro, therefore I'd like to
keep the code the way it is right now.

I also don't see why this could ever have the slightest chance of
"breaking everyone else".

For reference and easy viewing, here is the corresponding commit on
GitHub:
https://github.com/NetBSD/src/commit/ff11c7d3497a40c90ec70101ad72612e2f0884b2

Roland

On 2020/12/15 7:57, Christos Zoulas wrote:
In article <20201213212746.3CFC3FA9E%cvs.NetBSD.org@localhost>,
Roland Illig <source-changes-d%NetBSD.org@localhost> wrote:
-=-=-=-=-=-

Module Name:    src
Committed By:    rillig
Date:        Sun Dec 13 21:27:46 UTC 2020

Modified Files:
    src/usr.bin/make: for.c meta.c parse.c var.c

Log Message:
make(1): replace %zu with %u in printf calls

This is needed to compile bmake with GCC 2.8.1 on SunOS 5.9.

To support ancient systems like this, the whole code of usr.bin/make is
supposed to use only ISO C90 features, except for filemon, which is not
used on these systems.

Please revert! This breaks everyone else. %zu is the format to
print size_t.  Last I checked SunOS 5.9 has been dead since 2014
and whoever is still using it might as well install a new compiler,
or tie the box at the end of a long chain so it can find its true
calling. If you really want to support it instead define MAKE_FMT_SIZE_T
and conditionalize it properly for "ancient OS", windows, cygwin,
mingwin, and regular folks (this does not even handle "ancient os"):

     https://github.com/file/file/blob/master/src/file.h#L55

I am still against it though...

christos



Home | Main Index | Thread Index | Old Index