Re: 64-bit API for large object

2012/9/30 Tatsuo Ishii <ishii@postgresql.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@kaigai.gr.jp>