On 7/4/20 1:10 PM, Joe Conway wrote:
> On 7/4/20 12:52 PM, Tom Lane wrote:
>> Justin Pryzby writes:
>>> But I noticed that cfbot is now populating with failures like:
>>
>>> genfile.c: In function ‘read_binary_file’:
>>> genfile.c:192:5: error: ignoring return value of ‘fread’, declared with
On 7/4/20 12:52 PM, Tom Lane wrote:
> Justin Pryzby writes:
>> But I noticed that cfbot is now populating with failures like:
>
>> genfile.c: In function ‘read_binary_file’:
>> genfile.c:192:5: error: ignoring return value of ‘fread’, declared with
>> attribute warn_unused_result
Justin Pryzby writes:
> But I noticed that cfbot is now populating with failures like:
> genfile.c: In function ‘read_binary_file’:
> genfile.c:192:5: error: ignoring return value of ‘fread’, declared with
> attribute warn_unused_result [-Werror=unused-result]
> fread(rbuf, 1, 1, file);
>
Hi Joe
Thanks for addressing this.
But I noticed that cfbot is now populating with failures like:
https://travis-ci.org/github/postgresql-cfbot/postgresql/builds/704898559
genfile.c: In function ‘read_binary_file’:
genfile.c:192:5: error: ignoring return value of ‘fread’, declared with
On 7/2/20 6:29 PM, Tom Lane wrote:
> Joe Conway writes:
>> On 7/2/20 5:37 PM, Tom Lane wrote:
>>> I still can't get excited about contorting the code to remove that
>>> issue.
>
>> It doesn't seem much worse than the oom test that was there before -- see
>> attached.
>
> Personally I would not
Joe Conway writes:
> On 7/2/20 5:37 PM, Tom Lane wrote:
>> I still can't get excited about contorting the code to remove that
>> issue.
> It doesn't seem much worse than the oom test that was there before -- see
> attached.
Personally I would not bother, but it's your patch.
> Are we in
On 7/2/20 5:37 PM, Tom Lane wrote:
> Joe Conway writes:
>> In fact, in principle there is no reason we can't get to max - 4 with this
>> code
>> except that when the filesize is exactly 1073741819, we need to try to read
>> one
>> more byte to find the EOF that way I did in my patch. I.e.:
>
>
Joe Conway writes:
> On 7/2/20 4:27 PM, Tom Lane wrote:
>> Huh, I wonder why it's not max - 5. Probably not worth worrying about,
>> though.
> Well this part:
> + rbytes = fread(sbuf.data + sbuf.len, 1,
> +(size_t) (sbuf.maxlen - sbuf.len - 1), file);
> could actually be:
> +
On 7/2/20 4:27 PM, Tom Lane wrote:
> Joe Conway writes:
>> When I saw originally MaxAllocSize - 5 fail I skipped to something smaller by
>> 4096 and it worked. But here I see that the actual max size is MaxAllocSize
>> - 6.
>
> Huh, I wonder why it's not max - 5. Probably not worth worrying
Joe Conway writes:
> When I saw originally MaxAllocSize - 5 fail I skipped to something smaller by
> 4096 and it worked. But here I see that the actual max size is MaxAllocSize -
> 6.
Huh, I wonder why it's not max - 5. Probably not worth worrying about,
though.
On 7/2/20 3:36 PM, Tom Lane wrote:
> Joe Conway writes:
>> On 7/1/20 6:22 PM, Tom Lane wrote:
>>> Hm, I was expecting that the last successful iteration of
>>> enlargeStringInfo would increase the buffer size to MaxAllocSize,
>>> so that we'd really only be losing one byte (which we can't avoid
Joe Conway writes:
> On 7/1/20 6:22 PM, Tom Lane wrote:
>> Hm, I was expecting that the last successful iteration of
>> enlargeStringInfo would increase the buffer size to MaxAllocSize,
>> so that we'd really only be losing one byte (which we can't avoid
>> if we use stringinfo). But you're
On 7/1/20 6:22 PM, Tom Lane wrote:
> Joe Conway writes:
>> The only downside is that the max filesize is reduced to (MaxAllocSize -
>> MIN_READ_SIZE - 1) compared to MaxAllocSize with the old method.
>
> Hm, I was expecting that the last successful iteration of
> enlargeStringInfo would increase
Joe Conway writes:
> The only downside is that the max filesize is reduced to (MaxAllocSize -
> MIN_READ_SIZE - 1) compared to MaxAllocSize with the old method.
Hm, I was expecting that the last successful iteration of
enlargeStringInfo would increase the buffer size to MaxAllocSize,
so that
On 7/1/20 5:17 PM, Joe Conway wrote:
> On 7/1/20 4:12 PM, Tom Lane wrote:
>> Joe Conway writes:
>>> I did some performance testing of the worst case/largest possible file and
>>> found
>>> that skipping the stat and bulk read does cause a significant regression.
>>
>> Yeah, I was wondering a
On 7/1/20 4:12 PM, Tom Lane wrote:
> Joe Conway writes:
>> I did some performance testing of the worst case/largest possible file and
>> found
>> that skipping the stat and bulk read does cause a significant regression.
>
> Yeah, I was wondering a little bit if that'd be an issue.
>
>> In the
Joe Conway writes:
> I did some performance testing of the worst case/largest possible file and
> found
> that skipping the stat and bulk read does cause a significant regression.
Yeah, I was wondering a little bit if that'd be an issue.
> In the attached patch I was able to get most of the
On 6/28/20 6:00 PM, Tom Lane wrote:
> Joe Conway writes:
>> All good stuff -- I believe the attached checks all the boxes.
>
> Looks okay to me, except I think you want
>
> ! if (bytes_to_read > 0)
>
> to be
>
> ! if (bytes_to_read >= 0)
Yep -- thanks.
I did some performance testing
Joe Conway writes:
> All good stuff -- I believe the attached checks all the boxes.
Looks okay to me, except I think you want
! if (bytes_to_read > 0)
to be
! if (bytes_to_read >= 0)
As it stands, a zero request will be treated like -1 (read all the
rest of the file) while ISTM
On 6/27/20 3:43 PM, Tom Lane wrote:
> Joe Conway writes:
>> The attached patch fixes this for me. I think it ought to be backpatched
>> through
>> pg11.
>
>> Comments?
>
> 1. Doesn't seem to be accounting for the possibility of an error in fread().
>
> 2. Don't we want to remove the stat()
Joe Conway writes:
> The attached patch fixes this for me. I think it ought to be backpatched
> through
> pg11.
> Comments?
1. Doesn't seem to be accounting for the possibility of an error in fread().
2. Don't we want to remove the stat() call altogether, if we're not
going to believe its
Since pg11 pg_read_file() and friends can be used with absolute paths as long as
the user is superuser or explicitly granted the role pg_read_server_files.
I noticed that when trying to read a virtual file, e.g.:
SELECT pg_read_file('/proc/self/status');
the returned result is a zero length
22 matches
Mail list logo