Thread: Record comparison compiler warning
I am seeing this compiler warning in git head: rowtypes.c: In function 'record_image_cmp':rowtypes.c:1433: warning: 'cmpresult' may be useduninitialized in this functionrowtypes.c:1433: note: 'cmpresult' was declared here -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. +
Bruce Momjian <bruce@momjian.us> wrote: > I am seeing this compiler warning in git head: > > rowtypes.c: In function 'record_image_cmp': > rowtypes.c:1433: warning: 'cmpresult' may be used > uninitialized in this function rowtypes.c:1433: note: 'cmpresult' was declared here I had not gotten a warning under either gcc or clang, but that was probably because I was doing assert-enabled builds, and the Assert(false) saved me. That seemed a little marginal anyway, so how about this?: diff --git a/src/backend/utils/adt/rowtypes.c b/src/backend/utils/adt/rowtypes.c index ae007cf..0332593 100644 --- a/src/backend/utils/adt/rowtypes.c +++ b/src/backend/utils/adt/rowtypes.c @@ -1508,7 +1508,11 @@ record_image_cmp(FunctionCallInfo fcinfo) break; #endif default: - Assert(false); /* cannot happen */ + /* cannot happen */ + elog(ERROR, + "unexpected length of %i found comparing columns of type %s", + (int) tupdesc1->attrs[i1]->attlen, + format_type_be(tupdesc1->attrs[i1]->atttypid)); } } else Does that fix the warning for you? -- Kevin Grittner EDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Wed, Oct 16, 2013 at 11:49:13AM -0700, Kevin Grittner wrote: > Bruce Momjian <bruce@momjian.us> wrote: > > > I am seeing this compiler warning in git head: > > > > rowtypes.c: In function 'record_image_cmp': > > rowtypes.c:1433: warning: 'cmpresult' may be used > > uninitialized in this function rowtypes.c:1433: note: 'cmpresult' was declared here > > I had not gotten a warning under either gcc or clang, but that was > probably because I was doing assert-enabled builds, and the > Assert(false) saved me. That seemed a little marginal anyway, so > how about this?: Would you please send the file as ASCII, e.g. not: <A0><A0><A0><A0><A0><A0><A0><A0><A0><A0><A0><A0><A0><A0><A0><A0><A0><A0><A0> default: -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. +
Bruce Momjian <bruce@momjian.us> wrote: > On Wed, Oct 16, 2013 at 11:49:13AM -0700, Kevin Grittner wrote: >> Bruce Momjian <bruce@momjian.us> wrote: >> >>> I am seeing this compiler warning in git head: >>> >>> rowtypes.c: In function 'record_image_cmp': >>> rowtypes.c:1433: warning: 'cmpresult' may be used >>> uninitialized in this function rowtypes.c:1433: note: 'cmpresult' was declared here >> >> I had not gotten a warning under either gcc or clang, but that was >> probably because I was doing assert-enabled builds, and the >> Assert(false) saved me. That seemed a little marginal anyway, so >> how about this?: > > Would you please send the file as ASCII, e.g. not: > > <A0><A0><A0><A0><A0><A0><A0><A0><A0><A0><A0><A0><A0><A0><A0><A0><A0><A0><A0> default: Huh, I did not see anything remotely like that in my email or in the archives: http://www.postgresql.org/message-id/1381949353.78943.YahooMailNeo@web162902.mail.bf1.yahoo.com ... however, I'm attaching it rather than pasting it this time. It is warning-recimgcmp.diff. I'm also attaching the other warnings (outside of the well-known yyg parser warning) which I get from a compile using Ubuntu clang version 3.0-6ubuntu3. The one in pg_standby seems completely legitimate -- what is the point of comparing to see whether an unsigned integer is >= 0? The other one would be nice to quiet down, but I would understand if people would prefer to leave it alone and hope the compiler writers fix it. Warnings those fix below: catcache.c:571:12: warning: variable 'cl' is uninitialized when used here [-Wuninitialized] Assert(cl->cl_magic == CL_MAGIC); ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~ ../../../../src/include/c.h:625:10: note: expanded from: Trap(!(condition), "FailedAssertion") ^ ../../../../src/include/c.h:607:28: note: expanded from: if ((assert_enabled) && (condition)) \ ^~~~~~~~~ catcache.c:569:5: note: variable 'cl' is declared here CatCList *cl = dlist_container(CatCList, cache_elem, iter.cur); ^ catcache.c:585:13: warning: variable 'ct' is uninitialized when used here [-Wuninitialized] Assert(ct->ct_magic == CT_MAGIC); ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~ ../../../../src/include/c.h:625:10: note: expanded from: Trap(!(condition), "FailedAssertion") ^ ../../../../src/include/c.h:607:28: note: expanded from: if ((assert_enabled) && (condition)) \ ^~~~~~~~~ catcache.c:583:6: note: variable 'ct' is declared here CatCTup *ct = dlist_container(CatCTup, cache_elem, iter.cur); ^ 2 warnings generated. pg_standby.c:348:22: warning: comparison of unsigned expression >= 0 is always true [-Wtautological-compare] if (tli > 0 && log >= 0 && seg > 0) ~~~ ^ ~ 1 warning generated. None of these seem particularly urgent, so I'll leave them for comment for maybe a week before taking any action. -- Kevin Grittner EDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Attachment
On 10/31/2013 07:51 PM, Kevin Grittner wrote: > Bruce Momjian <bruce@momjian.us> wrote: >> On Wed, Oct 16, 2013 at 11:49:13AM -0700, Kevin Grittner wrote: > >>> Bruce Momjian <bruce@momjian.us> wrote: >>> >>>> I am seeing this compiler warning in git head: >>>> >>>> rowtypes.c: In function 'record_image_cmp': >>>> rowtypes.c:1433: warning: 'cmpresult' may be used >>>> uninitialized in this function rowtypes.c:1433: note: 'cmpresult' was declared here >>> >>> I had not gotten a warning under either gcc or clang, but that was >>> probably because I was doing assert-enabled builds, and the >>> Assert(false) saved me. That seemed a little marginal anyway, so >>> how about this?: >> >> Would you please send the file as ASCII, e.g. not: >> >> <A0><A0><A0><A0><A0><A0><A0><A0><A0><A0><A0><A0><A0><A0><A0><A0><A0><A0><A0> default: > > Huh, I did not see anything remotely like that in my email or in > the archives: > > http://www.postgresql.org/message-id/1381949353.78943.YahooMailNeo@web162902.mail.bf1.yahoo.com http://www.postgresql.org/message-id/raw/1381949353.78943.YahooMailNeo@web162902.mail.bf1.yahoo.com Stefan
Stefan Kaltenbrunner wrote: > On 10/31/2013 07:51 PM, Kevin Grittner wrote: > > Bruce Momjian <bruce@momjian.us> wrote: > >> On Wed, Oct 16, 2013 at 11:49:13AM -0700, Kevin Grittner wrote: > > > >>> Bruce Momjian <bruce@momjian.us> wrote: > >> Would you please send the file as ASCII, e.g. not: > >> > >> <A0><A0><A0><A0><A0><A0><A0><A0><A0><A0><A0><A0><A0><A0><A0><A0><A0><A0><A0> default: > > > > Huh, I did not see anything remotely like that in my email or in > > the archives: > > > > http://www.postgresql.org/message-id/1381949353.78943.YahooMailNeo@web162902.mail.bf1.yahoo.com > > http://www.postgresql.org/message-id/raw/1381949353.78943.YahooMailNeo@web162902.mail.bf1.yahoo.com I would blame Bruce's MUA, or surrounding configuration, for this problem. It looks fine in mine, and as far as I can see, Kevin's message correctly declares the email to be in Latin-1 quoted-printable encoding, which declares A0 to mean non-breaking space. Maybe, for instance, Bruce is running Mutt in Latin-1 mode with the terminal set to UTF-8 or some such? -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
Alvaro Herrera <alvherre@2ndquadrant.com> writes: > Stefan Kaltenbrunner wrote: >> http://www.postgresql.org/message-id/raw/1381949353.78943.YahooMailNeo@web162902.mail.bf1.yahoo.com > I would blame Bruce's MUA, or surrounding configuration, for this > problem. It looks fine in mine, and as far as I can see, Kevin's > message correctly declares the email to be in Latin-1 quoted-printable > encoding, which declares A0 to mean non-breaking space. I agree with Bruce: this patch is broken. A0 may be a non-breaking space, but the fact remains that it isn't a space, and since we're talking about a diff, the whitespace needs to be the same as what it is in the original file. (I believe that some of those whitespace runs involve tabs not just spaces, making the diff even more wrong.) Admittedly, if you're just eyeballing it, it might look fine. But try feeding it to "patch" and you'll find out it ain't. regards, tom lane