Thread: New warning in pg_dump

New warning in pg_dump

From
Alvaro Herrera
Date:
Hackers,

I'm seeing this:

gcc -O2 -fno-strict-aliasing -g -Wall -Wmissing-prototypes -Wmissing-declarations
-I/home/alvherre/CVS/pgsql/source/00orig/src/interfaces/libpq-I../../../src/include
-I/home/alvherre/CVS/pgsql/source/00orig/src/include-D_GNU_SOURCE  -DFRONTEND  -c -o pg_backup_db.o
/home/alvherre/CVS/pgsql/source/00orig/src/bin/pg_dump/pg_backup_db.c-MMD
 
/home/alvherre/CVS/pgsql/source/00orig/src/bin/pg_dump/pg_backup_db.c: In function `_isIdentChar':
/home/alvherre/CVS/pgsql/source/00orig/src/bin/pg_dump/pg_backup_db.c:874: warning: comparison is always true due to
limitedrange of data type
 
/home/alvherre/CVS/pgsql/source/00orig/src/bin/pg_dump/pg_backup_db.c: In function `_isDQChar':
/home/alvherre/CVS/pgsql/source/00orig/src/bin/pg_dump/pg_backup_db.c:891: warning: comparison is always true due to
limitedrange of data type
 

If I change _isIdentChar to be

static int _isIdentChar(unsigned char c)

I get instead

/home/alvherre/CVS/pgsql/source/00orig/src/bin/pg_dump/pg_backup_db.c: In function `_isIdentChar':
/home/alvherre/CVS/pgsql/source/00orig/src/bin/pg_dump/pg_backup_db.c:874: warning: comparison is always true due to
limitedrange of data type
 
/home/alvherre/CVS/pgsql/source/00orig/src/bin/pg_dump/pg_backup_db.c:874: warning: comparison is always false due to
limitedrange of data type
 

(yes, both lines), which is kind of strange.

This is

gcc (GCC) 3.4.1 (Mandrakelinux (Alpha 3.4.1-3mdk)

-- 
Alvaro Herrera (<alvherre[a]dcc.uchile.cl>)
"La fuerza no está en los medios físicos
sino que reside en una voluntad indomable" (Gandhi)



Re: New warning in pg_dump

From
Philip Warner
Date:
At 01:58 AM 24/08/2004, Alvaro Herrera wrote:
>static int _isIdentChar(unsigned char c)

I think the correct thing to do is to leave it as (signed) char, and remove 
the comparison to \200 = -127. All chars will be >= -127. I will fix this 
in the next release.


----------------------------------------------------------------
Philip Warner                    |     __---_____
Albatross Consulting Pty. Ltd.   |----/       -  \
(A.B.N. 75 008 659 498)          |          /(@)   ______---_
Tel: (+61) 0500 83 82 81         |                 _________  \
Fax: (+61) 03 5330 3172          |                 ___________ |
Http://www.rhyme.com.au          |                /           \|                                 |    --________--
PGP key available upon request,  |  /
and from pgp.mit.edu:11371       |/ 



Re: New warning in pg_dump

From
Philip Warner
Date:
At 01:27 PM 24/08/2004, Tom Lane wrote:
>I prefer declaring it as unsigned, which means you drop the
>\377 end instead ...

...I've used explicit values (128) since '\200' is -127.


----------------------------------------------------------------
Philip Warner                    |     __---_____
Albatross Consulting Pty. Ltd.   |----/       -  \
(A.B.N. 75 008 659 498)          |          /(@)   ______---_
Tel: (+61) 0500 83 82 81         |                 _________  \
Fax: (+61) 03 5330 3172          |                 ___________ |
Http://www.rhyme.com.au          |                /           \|                                 |    --________--
PGP key available upon request,  |  /
and from pgp.mit.edu:11371       |/  



Re: New warning in pg_dump

From
Tom Lane
Date:
Philip Warner <pjw@rhyme.com.au> writes:
> At 01:58 AM 24/08/2004, Alvaro Herrera wrote:
>> static int _isIdentChar(unsigned char c)

> I think the correct thing to do is to leave it as (signed) char, and remove 
> the comparison to \200 = -127.

No, that isn't the right thing, because not all platforms think char is
signed.  I prefer declaring it as unsigned, which means you drop the
\377 end instead ...
        regards, tom lane


Re: New warning in pg_dump

From
Philip Warner
Date:
At 01:27 PM 24/08/2004, Tom Lane wrote:
>I prefer declaring it as unsigned, which means you drop the
>\377 end instead ...

No problem.


----------------------------------------------------------------
Philip Warner                    |     __---_____
Albatross Consulting Pty. Ltd.   |----/       -  \
(A.B.N. 75 008 659 498)          |          /(@)   ______---_
Tel: (+61) 0500 83 82 81         |                 _________  \
Fax: (+61) 03 5330 3172          |                 ___________ |
Http://www.rhyme.com.au          |                /           \|                                 |    --________--
PGP key available upon request,  |  /
and from pgp.mit.edu:11371       |/ 



Re: New warning in pg_dump

From
Tom Lane
Date:
Philip Warner <pjw@rhyme.com.au> writes:
> At 01:27 PM 24/08/2004, Tom Lane wrote:
>> I prefer declaring it as unsigned, which means you drop the
>> \377 end instead ...

> ...I've used explicit values (128) since '\200' is -127.

Actually I'd go with "(unsigned char) '\200'".  There's a bunch
of different possible viewpoints here of course, but given that
you are trying to mimic a backend flex file that speaks of \200-\377,
I'd opt for something that's clearly traceable to that form.
        regards, tom lane