[9] RFR(L): 8037816 : Fix for 8036122 breaks build with Xcode5/clang

[David Chase]

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.

David