On Mon, 13 Apr 2026 at 02:51, Andres Freund <[email protected]> wrote: > I wonder if we should make at least "appending just the format string without > arguments" one permanently impossible for appendStringInfo(), instead of > playing whack-a-mole every release. Your wrapper macro
I'm open to considering having 0001 or portions of it in core. My patch isn't ideal, though. For example, if you look at what I had to do in xml.c. Maybe that wouldn't be quite as bad if I'd come up with a better name than appendStringInfoInternal(). There's also all the other cruddy changes in there, like WRITE_*_FIELD() stuff, which I had to make call the internal function to get it to compile. > extern void appendStringInfoInternal(StringInfo str, const char *fmt, ...) > pg_attribute_printf(2, 3); > #define appendStringInfo(str, fmt, ...) \ > appendStringInfoInternal(str, fmt, __VA_ARGS__) > > would give you compiler errors if you call appendStringInfo() without > an argument after fmt. It's not the greatest error message, but it'd probably > not confuse people too much? If we went and committed that macro, why wouldn't we include the StaticAssert part too? That'd also check for "%s". I suppose some compilers don't have __builtin_constant_p(), so I suppose it might be annoying if you test with one of those compilers and then only get a failure after committing. > Any reason you didn't have the relevant outfuncs.c fixes from 0001 - which > iiuc is not intended to be committed - in 0003? Pointlessly using > appendStringInfo() where appendStringInfoString() would have done there is > probably the by far most impactful misuse, performance wise. > > Specifically WRITE_CHAR_FIELD(), WRITE_FLOAT_FIELD() seem like they should use > appendStringInfoString()? We have a fair amount of both in node trees. I did miss those. I wrote the 0001 patch a long time ago and I don't remember noticing that those could be improved. I'm keen to fix those, but I'm just not sure that's valid post-freeze work since it's not touching new-to-v19 code. All the other stuff I did in 0003 is justified via it being all new code. David
