Re: [PATCH] PR pch/39492 - MinGW memory mapping fix

[Dave Korn <dave dot korn dot cygwin at googlemail dot com>]

Andrey Galkin wrote:
> Don't you feel such type of approach is a poor hack for this situation?

  No.  Using standard C library definitions and standard preprocessor and
language features to statically compute at compile-time a limit that
guarantees run-time safety and correctness is solid engineering.

  The fact that there were a couple of trivial typo/thinkos in the untested
first draft does not change that.

  The _actual_ fatal flaw in the scheme is in fact one that you didn't point out!

> Here are my points: * snprintf is already a safety measure - we won't get a
> corruption

  I take it you're referring to the use of computed field-size/precision
specifiers here?  I thought I had explained this more clearly than I obviously
did, as I failed to get the central point across: this is not there for
runtime safety.  Nor is it even there for compile-time safety.  It is there
for future-maintenance-work-safety.  It protects us against someone in the
future changing the printf format specifier without realising they'd need to
change the buffer size to match.  (Hopefully such a bug would be noticed in
patch review, but defensive programming protects you against the occasions
when things go /wrong/).  If you don't like it and aren't worried about
preventing a possible future accident, you could leave it out, or replace it
with a comment reminding people to keep the bufsize matched to the
printf-field size.

> * What you'll get by the macro is a length of "ULONG_MAX" string constant,
> but not the length of a substitution value (e.g. "4294967295" in the best
> case).

  That's just a trivial thinko, my suggestion was untested so I forgot I'd
need a second-layer of expansion to get the numeric value.  Here:

#define QUOTE2(x) #x
#define QUOTE(x) QUOTE2(x)

  That does what we want.

> * And, most likely, operation systems will keep the process ids below
> certain value. 

  And then next week Microsoft decide to start using the upper bits to store
some flags.  Or use negative PIDs.  Or whatever.  I think we should write the
code to work in all cases not just some cases that we hope will be the only
ones that ever arise, since it's not any harder for us and won't add
significantly to the time or memory consumption of the compiler.

* Even when GCC supports alloca, using strlen for a constant
> string, where sizeof-1 would give the same result is also a question.

  You're right again, another thinko, I of course meant to write sizeof where
I had strlen.  Hey, strlen won't get optimised at -O0, it's certainly not
suitable!

> IMO, this does not worth any perversion, unless there is a commonly used
> limits functionality.

  It's not perversion, it's just using the language features in a way that
they're designed to be used.  No, the actual fatal flaw is this: even with the
typos fixed, and even if you don't want the field size specified, there are
some systems that don't use a numeric constant for the definition but an
expression:

#define ULONG_MAX    (2147483647L * 2UL + 1)

  And that of course fails horribly.  Gah.  It's still a compile-time
constant, but it only becomes one after it's way too late to preprocess it.
Oh well.  Ah, hang on: maybe this is what you were hinting at when you said
"in the best case"?

> I've accented the hardcoded constant only to reduce questions. It wasn't
> clever from my side (:

  Hey, I did see that you were asking about if there were existing solutions,
but I thought I'd offer a possibility anyway, because if it had worked it
might have been an idea to adopt so that next time this happens there would be
an existing solution.  Anyway, since it can't work, your best option is just
to leave a comment explaining what's going on, as you already have.

    cheers,
      DaveK