Re: [HACKERS] Record comparison compiler warning

2013-11-02 Thread Stefan Kaltenbrunner
On 10/31/2013 07:51 PM, Kevin Grittner wrote:
 Bruce Momjian br...@momjian.us wrote:
 On Wed, Oct 16, 2013 at 11:49:13AM -0700, Kevin Grittner wrote:
 
 Bruce Momjian br...@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:

 A0A0A0A0A0A0A0A0A0A0A0A0A0A0A0A0A0A0A0 
 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.yahoomail...@web162902.mail.bf1.yahoo.com

http://www.postgresql.org/message-id/raw/1381949353.78943.yahoomail...@web162902.mail.bf1.yahoo.com


Stefan


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Record comparison compiler warning

2013-11-02 Thread Alvaro Herrera
Stefan Kaltenbrunner wrote:
 On 10/31/2013 07:51 PM, Kevin Grittner wrote:
  Bruce Momjian br...@momjian.us wrote:
  On Wed, Oct 16, 2013 at 11:49:13AM -0700, Kevin Grittner wrote:
  
  Bruce Momjian br...@momjian.us wrote:

  Would you please send the file as ASCII, e.g. not:
 
  A0A0A0A0A0A0A0A0A0A0A0A0A0A0A0A0A0A0A0
   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.yahoomail...@web162902.mail.bf1.yahoo.com
 
 http://www.postgresql.org/message-id/raw/1381949353.78943.yahoomail...@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 Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training  Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Record comparison compiler warning

2013-11-02 Thread Tom Lane
Alvaro Herrera alvhe...@2ndquadrant.com writes:
 Stefan Kaltenbrunner wrote:
 http://www.postgresql.org/message-id/raw/1381949353.78943.yahoomail...@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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Record comparison compiler warning

2013-10-31 Thread Kevin Grittner
Bruce Momjian br...@momjian.us wrote:
 On Wed, Oct 16, 2013 at 11:49:13AM -0700, Kevin Grittner wrote:

 Bruce Momjian br...@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:

 A0A0A0A0A0A0A0A0A0A0A0A0A0A0A0A0A0A0A0 
 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.yahoomail...@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 Companydiff --git a/src/backend/utils/adt/rowtypes.c b/src/backend/utils/adt/rowtypes.c
index cb04a72..516c37b 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
diff --git a/src/backend/utils/cache/catcache.c b/src/backend/utils/cache/catcache.c
index c467f11..89e6325 100644
--- a/src/backend/utils/cache/catcache.c
+++ b/src/backend/utils/cache/catcache.c
@@ -566,8 +566,9 @@ AtEOXact_CatCache(bool isCommit)
 			/* Check CatCLists */
 			dlist_foreach(iter, ccp-cc_lists)
 			{
-CatCList   *cl = dlist_container(CatCList, cache_elem, iter.cur);
+CatCList   *cl;
 
+cl = dlist_container(CatCList, cache_elem, iter.cur);
 Assert(cl-cl_magic == CL_MAGIC);
 Assert(cl-refcount == 0);
 Assert(!cl-dead);
@@ -580,8 +581,9 @@ AtEOXact_CatCache(bool isCommit)
 
 dlist_foreach(iter, bucket)
 {
-	CatCTup*ct = dlist_container(CatCTup, cache_elem, iter.cur);
+	CatCTup*ct;
 
+	ct = dlist_container(CatCTup, cache_elem, iter.cur);
 	Assert(ct-ct_magic == CT_MAGIC);
 	Assert(ct-refcount == 0);
 	Assert(!ct-dead);
diff --git a/contrib/pg_standby/pg_standby.c b/contrib/pg_standby/pg_standby.c
index a3f40fb..059c820 100644
--- a/contrib/pg_standby/pg_standby.c
+++ b/contrib/pg_standby/pg_standby.c
@@ -345,7 +345,7 @@ 

Re: [HACKERS] Record comparison compiler warning

2013-10-16 Thread Kevin Grittner
Bruce Momjian br...@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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Record comparison compiler warning

2013-10-16 Thread Bruce Momjian
On Wed, Oct 16, 2013 at 11:49:13AM -0700, Kevin Grittner wrote:
 Bruce Momjian br...@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:

A0A0A0A0A0A0A0A0A0A0A0A0A0A0A0A0A0A0A0 
default:

-- 
  Bruce Momjian  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

  + Everyone has their own god. +


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers