fix compile warning for pg_backup_tar.c

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