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

Michael Paquier <[email protected]> Mon, 9 Dec 2019 17:03:16 +0900

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

On Mon, Dec 09, 2019 at 08:49:26AM +0500, Andrey Lepikhov wrote:
> xact.c: In function ‘RecordTransactionAbort’:
> xact.c:5709:55: warning: argument 1 null where non-null expected [-Wnonnull]
>     XLogRegisterData(unconstify(char *, twophase_gid), strlen(twophase_gid)
> + 1);

    Assert(twophase_gid != NULL);
-
-   if (XLogLogicalInfoActive())
-       xl_xinfo.xinfo |= XACT_XINFO_HAS_GID;

xlinfo is set in the first part logging the transaction commit and the
record data is registered in the second, so I think that the original
coding makes more sense than what you are suggesting.  Perhaps it
would help to just add an assertion on twophase_gid to make sure that
it is not NULL in the part registering the data?  After that we really
have no bugs here, so it does not really help much..

> formatting.c: In function ‘parse_datetime’:
> formatting.c:4229:13: warning: ‘flags’ may be used uninitialized in this
> function [-Wmaybe-uninitialized]
>    if (flags & DCH_ZONED)

-   uint32      flags;
+   uint32      flags = 0;

    do_to_timestamp(date_txt, fmt, strict, &tm, &fsec, &fprec, &flags, have_error);

For this one, OK.  Wouldn't it be better to initialize flags, fprec
and have_error directly in do_to_timestamp if they are not NULL?  This
way future callers of the routine, if any, won't miss the
initialization.

By the way, are you using more specific CFLAGS to see that?  With -O3
and -Wnonnull I cannot spot both issues with GCC 9.2.1.
--
Michael

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