Thread: fix compile warning for pg_backup_tar.c
Yet another fix for a useless compile warning. This one is slightly ugly ;-(
cheers
andrew
Index: src/bin/pg_dump/pg_backup_tar.c
===================================================================
RCS file: /home/cvsmirror/pgsql/src/bin/pg_dump/pg_backup_tar.c,v
retrieving revision 1.45
diff -c -r1.45 pg_backup_tar.c
*** src/bin/pg_dump/pg_backup_tar.c 7 Oct 2004 15:21:55 -0000 1.45
--- src/bin/pg_dump/pg_backup_tar.c 8 Nov 2004 17:26:03 -0000
***************
*** 1019,1025 ****
--- 1019,1029 ----
*/
fseeko(tmp, 0, SEEK_END);
th->fileLen = ftello(tmp);
+ #ifdef INT64_IS_BUSTED
if (th->fileLen > MAX_TAR_MEMBER_FILELEN)
+ #else
+ if (((int64) th->fileLen -1) >= MAX_TAR_MEMBER_FILELEN)
+ #endif
die_horribly(AH, modulename, "archive member too large for tar format\n");
fseeko(tmp, 0, SEEK_SET);
Andrew Dunstan wrote:
> Yet another fix for a useless compile warning. This one is slightly
> ugly ;-(
First of all, ugly code needs to be documented in the code.
Then, the diff
+ #ifdef INT64_IS_BUSTED
if (th->fileLen > MAX_TAR_MEMBER_FILELEN)
+ #else
+ if (((int64) th->fileLen -1) >= MAX_TAR_MEMBER_FILELEN)
+ #endif
seems to imply that the current code corresponds to INT64_IS_BUSTED,
which obviously defies reason.
I remember when I wrote that code I consciously let the compile warning
stand for platforms with busted int64 because it became too confusing
to fix. Maybe you can clear that up for us. :)
--
Peter Eisentraut
http://developer.postgresql.org/~petere/
Andrew Dunstan <andrew@dunslane.net> writes:
> Yet another fix for a useless compile warning. This one is slightly ugly ;-(
Why not use off_t in the first place?
regards, tom lane
Peter Eisentraut wrote: >Andrew Dunstan wrote: > > >>Yet another fix for a useless compile warning. This one is slightly >>ugly ;-( >> >> > >First of all, ugly code needs to be documented in the code. > > Ok. Perhaps code that is expected to generate warnings should be too ;-) >Then, the diff > >+ #ifdef INT64_IS_BUSTED > if (th->fileLen > MAX_TAR_MEMBER_FILELEN) >+ #else >+ if (((int64) th->fileLen -1) >= MAX_TAR_MEMBER_FILELEN) >+ #endif > >seems to imply that the current code corresponds to INT64_IS_BUSTED, >which obviously defies reason. > >I remember when I wrote that code I consciously let the compile warning >stand for platforms with busted int64 because it became too confusing >to fix. Maybe you can clear that up for us. :) > > The error I saw was on Windows, for which I don't think int64 is busted, as we have long long int: pg_backup_tar.c:1022: warning: comparison is always false due to limited range of data type ISTM that what is happening here is that the compiler is smart enough to know that what is in MAX_TAR_MEMBER_FILELEN can't be exceeded by any possible value of type off_t. cheers andrew
Andrew Dunstan wrote: > ISTM that what is happening here is that the compiler is smart enough > to know that what is in MAX_TAR_MEMBER_FILELEN can't be exceeded by > any possible value of type off_t. Yeah, I think off_t is only 32 bits there. Then using INT64_IS_BUSTED as conditional is really misleading. -- Peter Eisentraut http://developer.postgresql.org/~petere/
Peter Eisentraut wrote: >Andrew Dunstan wrote: > > >>ISTM that what is happening here is that the compiler is smart enough >>to know that what is in MAX_TAR_MEMBER_FILELEN can't be exceeded by >>any possible value of type off_t. >> >> > >Yeah, I think off_t is only 32 bits there. Then using INT64_IS_BUSTED >as conditional is really misleading.\ > > What would you like me to use? Or maybe we should just do this in all cases instead of using an ifdef? cheers andrew
I have added a comment explaining the possible compiler warning:
/*
* Some compilers with throw a warning knowing this test can never be
* true because off_t can't exceed the compared maximum.
*/
if (th->fileLen > MAX_TAR_MEMBER_FILELEN)
die_horribly(AH, modulename, "archive member too large for tar format\n");
---------------------------------------------------------------------------
Andrew Dunstan wrote:
>
>
> Peter Eisentraut wrote:
>
> >Andrew Dunstan wrote:
> >
> >
> >>ISTM that what is happening here is that the compiler is smart enough
> >>to know that what is in MAX_TAR_MEMBER_FILELEN can't be exceeded by
> >>any possible value of type off_t.
> >>
> >>
> >
> >Yeah, I think off_t is only 32 bits there. Then using INT64_IS_BUSTED
> >as conditional is really misleading.\
> >
> >
>
> What would you like me to use? Or maybe we should just do this in all
> cases instead of using an ifdef?
>
> cheers
>
> andrew
>
> ---------------------------(end of broadcast)---------------------------
> TIP 4: Don't 'kill -9' the postmaster
>
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073