Horton

Re: Large writable variables

Andres Freund <andres@anarazel.de>

View thread View on postgr.es
From: Andres Freund <andres@anarazel.de>
To: Robert Haas <robertmhaas@gmail.com>
Cc: "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>, Thomas Munro <thomas.munro@enterprisedb.com>
Date: 2018-10-16T16:06:12Z
Lists: pgsql-hackers 
On 2018-10-16 11:22:31 -0400, Robert Haas wrote:
> On Tue, Oct 16, 2018 at 2:30 AM Andres Freund <andres@anarazel.de> wrote:
> > This just reminded me that a couple times I wanted a cast that casts
> > away const, but otherwise makes sure the type stays the same. I don't
> > think there's a way to do that in C, but we can write one that verifies
> > the cast doesn't do something bad if gcc is used:
> >
> > #if defined(HAVE__BUILTIN_TYPES_COMPATIBLE_P)
> > #define unconstify(cst, var) StaticAssertExpr(__builtin_types_compatible_p(__typeof(var), const cst), "wrong cast"), (cst) (var)
> > #else
> > #define unconstify(cst, var) ((cst) (var))
> > #endif
> >
> > Does anybody besides me see value in adding a cleaned up version of
> > that?
> 
> Under what circumstances would we consider this to be a legitimate thing to use?

When the variable actually *will not* be modified, but language or API
design reasons makes it unfeasiable to express that. Look e.g.
DestReceiver * CreateDestReceiver(CommandDest dest);
some of the returned receivers (e.g. donothingDR, printsimpleDR) are
statically allocated and *any* modification would be a bug. But other
return values will be modified, e.g. CreateIntoRelDestReceiver().

It's safe to cast constness away if the variable will not actually be
modified after. Which is e.g. the case above. But making the static
allocations const will a) save memory b) trigger sigbuses if you modify
them. So the casting constness away here *increases* robustness.

The problem is that just adding a cast like
		case DestNone:
			return (DestReceiver *) &donothingDR;
also hides errors. If you e.g. changed the type of donothingDR you'd
still not get an error.

So I was wishing for a form of a cast that only casts the const away,
but errors out if there's any other type difference. That's the above, I
think.


> I think if we add something this, we'd better accompany it with some
> detailed and very clearly-written statements about when you're allowed
> to use it.  Otherwise, I predict that people will use it in cases
> where it's not actually safe, and we'll end up with low-grade bugs.

Well, right now people will (and have) just cast the const away like
above. So I don't really see it being more likely to cause problems than
we're doing now.  But yea, it definitely should have a big red warning
label.

Greetings,

Andres Freund

Commits

  1. Apply unconstify() in more places

  2. Improve unconstify() documentation

  3. Drop const cast from dlsym() calls

  4. Const-ify a few more large static tables.

  5. Improve tzparse's handling of TZDEFRULES ("posixrules") zone data.

  6. Avoid statically allocating statement cache in ecpglib/prepare.c.

  7. Reorder FmgrBuiltin members, saving 25% in size.

  8. Add macro to cast away const without allowing changes to underlying type.

  9. Mark constantly allocated dest receiver as const.

  10. Avoid statically allocating formatting.c's format string caches.

  11. Correct constness of system attributes in heap.c & prerequisites.

  12. Avoid statically allocating gmtsub()'s timezone workspace.

  13. Correct constness of a few variables.

  14. Move the replication lag tracker into heap memory.