Horton

Re: GCC 8 warnings

Andres Freund <andres@anarazel.de>

View thread View on postgr.es
From: Andres Freund <andres@anarazel.de>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>, Devrim Gündüz <devrim@gunduz.org>, pgsql-hackers <pgsql-hackers@lists.postgresql.org>
Date: 2018-04-28T16:41:35Z
Lists: pgsql-hackers 
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

Commits

  1. Use -Wno-format-truncation and -Wno-stringop-truncation, if available.

  2. Avoid unnecessary use of strncpy in a couple of places in ecpg.

  3. Use snprintf not sprintf in pg_waldump's timestamptz_to_str.