Re: 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 used
uninitialized in this function rowtypes.c:1433: note: 'cmpresult' was declared here

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +

Bruce Momjian <bruce(at)momjian(dot)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(at)momjian(dot)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(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +

Bruce Momjian <bruce(at)momjian(dot)us> wrote:
> On Wed, Oct 16, 2013 at 11:49:13AM -0700, Kevin Grittner wrote:

>> Bruce Momjian <bruce(at)momjian(dot)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

On 10/31/2013 07:51 PM, Kevin Grittner wrote:
> Bruce Momjian <bruce(at)momjian(dot)us> wrote:
>> On Wed, Oct 16, 2013 at 11:49:13AM -0700, Kevin Grittner wrote:
>
>>> Bruce Momjian <bruce(at)momjian(dot)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(dot)78943(dot)YahooMailNeo(at)web162902(dot)mail(dot)bf1(dot)yahoo(dot)com

Stefan

Stefan Kaltenbrunner wrote:
> On 10/31/2013 07:51 PM, Kevin Grittner wrote:
> > Bruce Momjian <bruce(at)momjian(dot)us> wrote:
> >> On Wed, Oct 16, 2013 at 11:49:13AM -0700, Kevin Grittner wrote:
> >
> >>> Bruce Momjian <bruce(at)momjian(dot)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(dot)78943(dot)YahooMailNeo(at)web162902(dot)mail(dot)bf1(dot)yahoo(dot)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(at)2ndquadrant(dot)com> writes:
> Stefan Kaltenbrunner wrote:
>> http://www.postgresql.org/message-id/raw/1381949353(dot)78943(dot)YahooMailNeo(at)web162902(dot)mail(dot)bf1(dot)yahoo(dot)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