Re: Warning in the RecordTransactionAbort routine during compilation with O3 flag

Michael Paquier <[email protected]> Tue, 10 Dec 2019 12:13:28 +0900

Newsgroups	gmane.comp.db.postgresql.bugs
Message-ID	<[email protected]>

On Mon, Dec 09, 2019 at 02:03:43PM +0500, Andrey Lepikhov wrote:
> We already have assertion on the twophase_gid variable. But compiler is not
> so smart and can't find a link between the XACT_XINFO_HAS_GID flag and
> state of twophase_gid pointer.

Well, gcc-9 got visibly smarter on that point :)

> Ok. In accordance with your review, I have prepared a new version of the
> patch.

Regarding formatting.c, I can see the point of avoiding future
mistakes, and I would go a bit further as per the attached for
consistency between all variables.  have_error is a bit trickier
though as it gets moved around more layers so doing an initialization
in the middle is not really an option.  Anyway, we can do that rather
cleanly from the entry point of do_to_timestamp() to bring more
consistency for variables which are always expected and the optional
ones.  What do you think?

For the second one in xact.c, I am not really on board of doing
something based on the proposals because this reduces the code
visibility, and gcc is clearly wrong in its assumptions because the
state cannot be reached.

> My compiler:
> 
> gcc -v
> [...]
> gcc version 7.4.0 (Ubuntu 7.4.0-1ubuntu1~18.04.1)

Thanks, that's the difference.  gcc-9 does not complain, but I can see
the warnings with gcc-7 (7.5.0 actually).
--
xMichael

tmsp-formatting-warnings.patch (text/x-diff, 867 B)

diff --git a/src/backend/utils/adt/formatting.c b/src/backend/utils/adt/formatting.c
index f7175df8da..8fcbc2267f 100644
--- a/src/backend/utils/adt/formatting.c
+++ b/src/backend/utils/adt/formatting.c
@@ -4128,7 +4128,7 @@ parse_datetime(text *date_txt, text *fmt, bool strict, Oid *typid,
 {
 	struct pg_tm tm;
 	fsec_t		fsec;
-	int			fprec = 0;
+	int			fprec;
 	uint32		flags;
 
 	do_to_timestamp(date_txt, fmt, strict, &tm, &fsec, &fprec, &flags, have_error);
@@ -4318,11 +4318,18 @@ do_to_timestamp(text *date_txt, text *fmt, bool std,
 	int			fmask;
 	bool		incache = false;
 
+	Assert(tm != NULL);
+	Assert(fsec != NULL);
+
 	date_str = text_to_cstring(date_txt);
 
 	ZERO_tmfc(&tmfc);
 	ZERO_tm(tm);
 	*fsec = 0;
+	if (fprec)
+		*fprec = 0;
+	if (flags)
+		*flags = 0;
 	fmask = 0;					/* bit mask for ValidateDate() */
 
 	fmt_len = VARSIZE_ANY_EXHDR(fmt);

signature.asc (application/pgp-signature, 833 B)