RFR(L): 8037816 : Fix for 8036122 breaks build with Xcode5/clang
david.r.chase at oracle.com
Fri Apr 25 01:48:30 UTC 2014
On 2014-04-24, at 9:04 PM, John Rose <john.r.rose at oracle.com> wrote:
> On Apr 24, 2014, at 1:49 PM, David Chase <david.r.chase at oracle.com> wrote:
>> (3) these are the parts/questions I think need opinions/resolution:
>> (a) print_raw(foo) vs print(“%s”, foo)?
TBD, pending other comments, I think.
>> (b) print_cr() vs print_raw_cr(“”) vs print_cr(“%s”, “”) vs cr().
>> Note that some of our format-related constructors demand a format, hence they go
>> with print("%s", "") when they start empty.
> Applying the above reasoning, I do not support introducing "print_cr()" as an alias for "cr()". If we don't like the name of "cr()" we should change it in a cleanup. Adding an alias is (a) API tweaking, and (b) leads to pseudo-information in the code which will need cleaning later.
> (By pseudo-information, I mean what amounts to a free choice between cr and print_cr injecting a random bit at each use point, and causing future programmers to muse, "he must have a reason for making the distinction; some tool or audience must need that information".)
I know exactly what you mean.
I did not remove prints of the empty string because someone must have had some reason for doing this, right?
I'll replace all instances of print_cr() with cr() and remove print_cr().
>> I tested the various pragma/macros idioms for clang as far back as 3.1, gcc as far back as 4.2 .
>> Added some formatting macros:
>> #define UINT64_FORMAT_X "%" PRIx64
>> #define PTR_FORMAT_W(width) "%" #width PRIxPTR
>> #define SIZE_FORMAT_XW(width) "%" #width PRIxPTR
>> #define SIZE_FORMAT_X "%" PRIxPTR
> A good step. That large herd of macros may need further taming later. Your thoughts on this (now that it's all in your head) would be welcome.
I don't know for sure; it seemed like a hodge-podge.
I'll think some more.
>> + warning("Failed to reserve shared memory (errno = %d).", errno);
> I'm not sure what to do with the snippets at the end of your note, except to say I agree that it is reasonable to insert "(int)" casts for arguments to "%d", at least when there is reason to believe that the programmer would accept a format string containing a truncated value. The presence of "%d" strongly suggests the programmer expects a small int; same with "%u". And truncating the value does not add a bug; at most the risk is of continuing to hide a smaller bug.
I've gone back and forth on this; on the one hand casting to (int) for a %d format "doesn't add any new truncation", but it gives the impression of intent. I was a little unsure whether we ever need to worry about dealing with more than 2 trillion bytes (a billion K) of storage, but it seemed remotely possible, so I actually backed out a bunch of changes for %d K and converted them to UINTX_FORMAT K or SIZE_FORMAT K, depending on the type coming in.
More information about the hotspot-dev