Prev: [HACKERS] pg_dump's checkSeek() seems inadequate
Next: [HACKERS] suppress automatic recovery after back crash
From: Tom Lane on 27 Jun 2010 14:33
I wrote:
> The test that checkSeek() is using is to see whether this works:
> fseeko(fp, 0, SEEK_CUR)
> and apparently on this platform that's a no-op even on an otherwise
> unseekable file.

BTW, I looked in the archives for related discussions and found only the
thread in which the current fseek-checking code was designed. There
were a couple of mentions that SEEK_CUR should be changed to SEEK_SET, eg
http://archives.postgresql.org/pgsql-hackers/2002-10/msg01088.php
but it was apparently focused around coping with specific seek APIs
that didn't have SEEK_CUR at all. We ended up not using those so the
change was never made. The thought that a no-op seek might not give
the right answer wasn't discussed AFAICT.

regards, tom lane

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

From: Robert Haas on 27 Jun 2010 18:05
On Sun, Jun 27, 2010 at 1:42 PM, Tom Lane <tgl(a)sss.pgh.pa.us> wrote:
> If I change the test to be
>
> � � � �fseeko(fp, 0, SEEK_SET)
>
> then it does the right thing. �Since checkSeek() is applied immediately
> after opening the input file this should be OK, but it does limit the
> scope of usefulness of that function.
>
> Any thoughts about whether or not to apply such a patch? �If it should
> be changed, should we back-patch it?

Well, I guess it depends on what you think the chances are that the
revised test will fail on some other obscure platform. Have there
been any reports from the field? If not, I might apply to HEAD and
await developments.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

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

From: Tom Lane on 27 Jun 2010 18:19
Robert Haas <robertmhaas(a)gmail.com> writes:
> On Sun, Jun 27, 2010 at 1:42 PM, Tom Lane <tgl(a)sss.pgh.pa.us> wrote:
>> If I change the test to be
>> fseeko(fp, 0, SEEK_SET)
>> then it does the right thing.

> Well, I guess it depends on what you think the chances are that the
> revised test will fail on some other obscure platform.

To believe that, you'd have to believe that fseeko(fp, 0, SEEK_SET)
will fail but fseeko(fp, something-not-zero, SEEK_SET) will succeed.

A somewhat more plausible scenario is that somebody might hope that
they could do something like this:

echo 'some custom header' >pg.dump
pg_dump -Fc >>pg.dump

I believe that (at least on most Unixen) doing fseeko(fp, 0, SEEK_SET)
would result in overwriting the custom header, where it would not have
been overwritten before. However the usefulness of the above is at
best far-fetched; and I'm not very sure that it works today anyway,
since pg_dump/pg_restore seem to assume that manual byte counting should
match the results of ftell().

regards, tom lane

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

From: Robert Haas on 27 Jun 2010 20:42
On Sun, Jun 27, 2010 at 6:19 PM, Tom Lane <tgl(a)sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas(a)gmail.com> writes:
>> On Sun, Jun 27, 2010 at 1:42 PM, Tom Lane <tgl(a)sss.pgh.pa.us> wrote:
>>> If I change the test to be
>>> � � � �fseeko(fp, 0, SEEK_SET)
>>> then it does the right thing.
>
>> Well, I guess it depends on what you think the chances are that the
>> revised test will fail on some other obscure platform.
>
> To believe that, you'd have to believe that fseeko(fp, 0, SEEK_SET)
> will fail but fseeko(fp, something-not-zero, SEEK_SET) will succeed.
>
> A somewhat more plausible scenario is that somebody might hope that
> they could do something like this:
>
> � � � �echo 'some custom header' >pg.dump
> � � � �pg_dump -Fc >>pg.dump
>
> I believe that (at least on most Unixen) doing fseeko(fp, 0, SEEK_SET)
> would result in overwriting the custom header, where it would not have
> been overwritten before. �However the usefulness of the above is at
> best far-fetched; and I'm not very sure that it works today anyway,
> since pg_dump/pg_restore seem to assume that manual byte counting should
> match the results of ftell().

That doesn't actually sound all that far-fetched.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

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

From: Tom Lane on 27 Jun 2010 20:55
Robert Haas <robertmhaas(a)gmail.com> writes:
> On Sun, Jun 27, 2010 at 6:19 PM, Tom Lane <tgl(a)sss.pgh.pa.us> wrote:
>> A somewhat more plausible scenario is that somebody might hope that
>> they could do something like this:
>>
>> echo 'some custom header' >pg.dump
>> pg_dump -Fc >>pg.dump

> That doesn't actually sound all that far-fetched.

I've got my doubts, but I guess we could write an even-more-bulletproof
test by doing ftello() and then seeing if fseeko to that position
complains. This might be a good thing anyway since most of the other
uses of ftello aren't really checking for errors ...

regards, tom lane

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

 |  Next  |  Last
Pages: 1 2
Prev: [HACKERS] pg_dump's checkSeek() seems inadequate
Next: [HACKERS] suppress automatic recovery after back crash