|To:||"Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us>|
|Cc:||ashutosh(dot)bapat <ashutosh(dot)bapat(at)2ndquadrant(dot)com>, pgsql-hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>|
|Subject:||Re: A bug when use get_bit() function for a long bytea string|
|Views:||Raw Message | Whole Thread | Download mbox | Resend email|
>I don't think this has really solved the overflow hazards. For example,
>in binary_encode we've got
>resultlen = enc->encode_len(VARDATA_ANY(data), datalen);
>result = palloc(VARHDRSZ + resultlen);
>and all you've done about that is changed resultlen from int to int64.
>On a 64-bit machine, sure, palloc will be able to detect if the
>result exceeds what can be allocated --- but on a 32-bit machine
>it'd be possible for the size_t argument to overflow altogether.
>(Or if you want to argue that it can't overflow because no encoder
>expands the data more than 4X, then we don't need to be making this
>change at all.)
>I don't think there's really any way to do that safely without an
>explicit check before we call palloc.
I am sorry I do not very understand these words, and especially
what's the mean by 'size_t'.
Here I change resultlen from int to int64, is because we can get a right
error report value other than '-1' or another strange number.
>Why did you change the outputs from unsigned to signed? Why didn't
>you change the dlen inputs? I grant that we know that the input
>can't exceed 1GB in Postgres' usage, but these signatures are just
>randomly inconsistent, and you didn't even add a comment explaining
>why. Personally I think I'd make them like
>uint64 (*encode_len) (const char *data, size_t dlen);
>which makes it explicit that the dlen argument describes the length
>of a chunk of allocated memory, while the result might exceed that.
I think it makes sence and followed.
>Lastly, there is a component of this that can be back-patched and
>a component that can't --- we do not change system catalog contents
>in released branches. Cramming both parts into the same patch
>is forcing the committer to pull them apart, which is kind of
Sorry about that, attached is the changed patch for PG13, and the one
for older branches will send sooner.
Highgo Software (Canada/China/Pakistan)
URL : www.highgo.ca
|Next Message||Julien Rouhaud||2020-04-02 06:29:31||Re: User Interface for WAL usage data|
|Previous Message||Michael Paquier||2020-04-02 06:21:50||Re: potential stuck lock in SaveSlotToPath()|