Re: 64-bit API for large object

From: Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp>
To: Tatsuo Ishii <ishii(at)postgresql(dot)org>
Cc: anzai(at)sraoss(dot)co(dot)jp, pgsql-hackers(at)postgresql(dot)org
Subject: Re: 64-bit API for large object
Date: 2012-09-30 10:20:27
Message-ID: CADyhKSVb6h+2+j4d5DB5Bo3piXPsrDYLwcYon+aKbvqt9FNUJA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

2012/9/30 Tatsuo Ishii <ishii(at)postgresql(dot)org>:
>> * At inv_seek(), it seems to me it checks offset correctness with wrong way,
>> as follows:
>> | case SEEK_SET:
>> | if (offset < 0)
>> | elog(ERROR, "invalid seek offset: " INT64_FORMAT, offset);
>> | obj_desc->offset = offset;
>> | break;
>> It is a right assumption, if large object size would be restricted to 2GB.
>> But the largest positive int64 is larger than expected limitation.
>> So, it seems to me it should be compared with (INT_MAX * PAGE_SIZE)
>> instead.
>
> Point taken. However, checking offset < 0 seems to be still valid
> because it is possible to pass minus offset to inv_seek(), no? Also I
> think upper limit for seek position should be defined as (INT_MAX *
> LOBLKSIZE), rather than (INT_MAX * PAGE_SIZE). Probably (INT_MAX *
> LOBLKSIZE) should be defined in pg_largeobject.h as:
>
> /*
> * Maximum byte length for each large object
> */
> #define MAX_LARGE_OBJECT_SIZE INT64CONST(INT_MAX * LOBLKSIZE)
>
> Then the checking offset in inv_seek() will be:
>
> case SEEK_SET:
> if (offset < 0 || offset >= MAX_LARGE_OBJECT_SIZE)
> elog(ERROR, "invalid seek offset: " INT64_FORMAT, offset);
> obj_desc->offset = offset;
> break;
> case SEEK_CUR:
> if ((offset + obj_desc->offset) < 0 ||
> (offset + obj_desc->offset) >= MAX_LARGE_OBJECT_SIZE)
> elog(ERROR, "invalid seek offset: " INT64_FORMAT, offset);
> obj_desc->offset += offset;
> break;
> case SEEK_END:
> {
> int64 pos = inv_getsize(obj_desc) + offset;
>
> if (pos < 0 || pos >= MAX_LARGE_OBJECT_SIZE)
> elog(ERROR, "invalid seek offset: " INT64_FORMAT, offset);
> obj_desc->offset = pos;
> }
>
> What do you think?
>
Yes, it is exactly what I expected. Indeed, it is still need a check to prevent
negative offset here.

Thanks,
--
KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Thomas Munro 2012-09-30 10:21:01 pg_regress running for ~10 hours using 100% CPU
Previous Message Tatsuo Ishii 2012-09-30 09:26:37 Question regarding Sync message and unnamed portal