Re: GCC 8 warnings

Hi,

On 2018-04-28 12:16:39 -0400, Tom Lane wrote:
> -SharedFilePath(char *path, SharedFileSet *fileset, const char *name)
> +SharedFilePath(char *path, size_t size, SharedFileSet *fileset, const char *name)
> {
> - char dirpath[MAXPGPATH];
> + char dirpath[MAXPGPATH + 100];
>
> - SharedFileSetPath(dirpath, fileset, ChooseTablespace(fileset, name));
> - snprintf(path, MAXPGPATH, "%s/%s", dirpath, name);
> + SharedFileSetPath(dirpath, sizeof(dirpath), fileset, ChooseTablespace(fileset, name));
> + snprintf(path, size, "%s/%s", dirpath, name);
> }
>
> we basically haven't got any coherent strategy at all, other than
> "whack it till GCC 8 stops complaining". Some of these strings
> might be longer than MAXPGPATH, and we're not very clear on which.
> Worse, the recursive rule that paths are shorter than MAXPGPATH has
> collapsed entirely, so that any reasoning on the assumption that the
> input strings are shorter than MAXPGPATH is now suspect as can be.

> So basically, I think that any argument that these changes make us
> more secure against unwanted path truncation is just horsepucky.
> There's no longer any clear understanding of the maximum supported
> string length, and without global analysis of that you can't say
> that truncation will never happen.

+1

> Perhaps there's an argument for trying to get rid of MAXPGPATH
> altogether, replacing *all* of these fixed-length buffers with
> psprintf-type coding. I kinda doubt it's worth the trouble,
> but if somebody wants to work on it I wouldn't say no.

Same.

> In the meantime, I think our response to GCC 8 should just be to
> enable -Wno-format-truncation. Certainly so in the back branches.
> There isn't one of these changes that is actually fixing a bug,
> which to me says that that warning is unhelpful.

Agreed. Or at least turn down its aggressiveness to the cases where it's
actually sure truncation happens.

Greetings,

Andres Freund