Re: Simplified version of read_binary_file (src/backend/utils/adt/genfile.c) - Mailing list pgsql-hackers

From Tom Lane
Subject Re: Simplified version of read_binary_file (src/backend/utils/adt/genfile.c)
Date
Msg-id 473732.1599857065@sss.pgh.pa.us
Whole thread Raw
In response to Re: Simplified version of read_binary_file (src/backend/utils/adt/genfile.c)  (Ranier Vilela <ranier.vf@gmail.com>)
Responses Re: Simplified version of read_binary_file (src/backend/utils/adt/genfile.c)  (Ranier Vilela <ranier.vf@gmail.com>)
Re: Simplified version of read_binary_file (src/backend/utils/adt/genfile.c)  (Ranier Vilela <ranier.vf@gmail.com>)
List pgsql-hackers
Ranier Vilela <ranier.vf@gmail.com> writes:
> New version, with support to read Virtual File (pipe, FIFO and socket).
> With assert, in case, erroneous, of trying to read a pipe, with offset.

Really, could you do a little more thinking and testing of your own,
rather than expecting the rest of us to point out holes in your thinking?

* bytes_to_read == 0 is a legal case.

* "Assert(seek_offset != 0)" is an equally awful idea.

* Removing the seek code from the negative-bytes_to_read path
is just broken.

* The only reason this code is shorter than the previous version is
you took out all the comments explaining what it was doing, and
failed to replace them.  This is just as subtle as before, so I
don't find that acceptable.

I do agree that it might be worth skipping the fseeko call in the
probably-common case where seek_offset is zero.  Otherwise I don't
see much reason to change what's there.

            regards, tom lane



pgsql-hackers by date:

Previous
From: Alvaro Herrera
Date:
Subject: Re: problem with RETURNING and update row movement
Next
From: Tom Lane
Date:
Subject: Re: factorial function/phase out postfix operators?