Thread: Incorrect comment in fe-lobj.c

I found following in fe-lobj.c:

/** lo_lseek*      change the current read or write location on a large object* currently, only L_SET is a legal value
forwhence**/
 

I don't know where "L_SET" comes from. Anyway this should be:* whence must be one of SEEK_SET, SEEK_CUR or SEEK_END.

If there's no objection, I will change this.
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese: http://www.sraoss.co.jp

Tatsuo Ishii <ishii@postgresql.org> writes:
> I found following in fe-lobj.c:

>  * currently, only L_SET is a legal value for whence

> I don't know where "L_SET" comes from.

Hmm, seems to be that way in the original commit to our CVS (Postgres95).
I don't find this code at all in Postgres v4r2 though.

> Anyway this should be:
>  * whence must be one of SEEK_SET, SEEK_CUR or SEEK_END.

Agreed.  But looking at this brings a thought to mind: our code is
assuming that SEEK_SET, SEEK_CUR, SEEK_END have identical values on the
client and server.  The lack of complaints over the past fifteen years
suggests that every Unix-oid platform is in fact using the same values
for these macros ... but that seems kind of a risky assumption.  Is it
worth changing?  And if so, how would we go about that?
        regards, tom lane

> Tatsuo Ishii <ishii@postgresql.org> writes:
>> I found following in fe-lobj.c:
> 
>>  * currently, only L_SET is a legal value for whence
> 
>> I don't know where "L_SET" comes from.
> 
> Hmm, seems to be that way in the original commit to our CVS (Postgres95).
> I don't find this code at all in Postgres v4r2 though.

I just remembered that "L_SET" came from old BSDish systems.

>> Anyway this should be:
>>  * whence must be one of SEEK_SET, SEEK_CUR or SEEK_END.
> 
> Agreed.  But looking at this brings a thought to mind: our code is
> assuming that SEEK_SET, SEEK_CUR, SEEK_END have identical values on the
> client and server.  The lack of complaints over the past fifteen years
> suggests that every Unix-oid platform is in fact using the same values
> for these macros ... but that seems kind of a risky assumption.  Is it
> worth changing?  And if so, how would we go about that?

I personaly have not seen any definitions other than below before.

# define SEEK_SET    0    /* Seek from beginning of file.  */
# define SEEK_CUR    1    /* Seek from current position.  */
# define SEEK_END    2    /* Seek from end of file.  */

However I agree your point. What about defining our own definitions
which have exact same values as above? i.e.;

# define PG_SEEK_SET    0    /* Seek from beginning of file.  */
# define PG_SEEK_CUR    1    /* Seek from current position.  */
# define PG_SEEK_END    2    /* Seek from end of file.  */
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese: http://www.sraoss.co.jp

Tatsuo Ishii <ishii@postgresql.org> writes:
>> Agreed.  But looking at this brings a thought to mind: our code is
>> assuming that SEEK_SET, SEEK_CUR, SEEK_END have identical values on the
>> client and server.  The lack of complaints over the past fifteen years
>> suggests that every Unix-oid platform is in fact using the same values
>> for these macros ... but that seems kind of a risky assumption.  Is it
>> worth changing?  And if so, how would we go about that?

> I personaly have not seen any definitions other than below before.

> # define SEEK_SET    0    /* Seek from beginning of file.  */
> # define SEEK_CUR    1    /* Seek from current position.  */
> # define SEEK_END    2    /* Seek from end of file.  */

Same here.

> However I agree your point. What about defining our own definitions
> which have exact same values as above? i.e.;

> # define PG_SEEK_SET    0    /* Seek from beginning of file.  */
> # define PG_SEEK_CUR    1    /* Seek from current position.  */
> # define PG_SEEK_END    2    /* Seek from end of file.  */

Well, the thing is: if all platforms use those same values, then this is
a pretty useless change (and yet one that affects client applications,
not only our own code).  If not all platforms use those values, then
this is a wire-protocol break for those that don't.

Is there a way to fix things so that we don't have a protocol break?
I think it's clearly impossible across platforms that have inconsistent
SEEK_XXX definitions; but such cases were incompatible before anyway.
Can we make it not break if client and server are the same platform,
but have some other set of SEEK_XXX values?
        regards, tom lane