thinko in convertToJsonb()

Started by Mark Dilgerabout 11 years ago3 messages
#1Mark Dilger
mark@port25.com

The call:

reserveFromBuffer(&buffer, sizeof(VARHDRSZ))

is assuming that the size of varlena header is the same
size as the type used to return that size, which happens
to be so, but someone could easily change that macro
to:

#define VARHDRSZ ((int64) sizeof(int32))

And you'd want to reserve sizeof(int32), not sizeof(int64)
in convertToJsonb.

Perhaps the code really meant to say:

reserveFromBuffer(&buffer, VARHDRSZ)

mark

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#2Michael Paquier
michael.paquier@gmail.com
In reply to: Mark Dilger (#1)
1 attachment(s)
Re: thinko in convertToJsonb()

On Tue, Dec 9, 2014 at 11:11 AM, Mark Dilger <mark@port25.com> wrote:

The call:

reserveFromBuffer(&buffer, sizeof(VARHDRSZ))

is assuming that the size of varlena header is the same
size as the type used to return that size, which happens
to be so, but someone could easily change that macro
to:

#define VARHDRSZ ((int64) sizeof(int32))

And you'd want to reserve sizeof(int32), not sizeof(int64)
in convertToJsonb.

Perhaps the code really meant to say:

reserveFromBuffer(&buffer, VARHDRSZ)

Good catch! The code is indeed incorrect. Attached is a one-line patch
addressing that, I guess someone is going to pick up that sooner or
later.
--
Michael

Attachments:

20141210_jsonb_varlena_header.patchapplication/octet-stream; name=20141210_jsonb_varlena_header.patchDownload
diff --git a/src/backend/utils/adt/jsonb_util.c b/src/backend/utils/adt/jsonb_util.c
index 2ff8539..c62941b 100644
--- a/src/backend/utils/adt/jsonb_util.c
+++ b/src/backend/utils/adt/jsonb_util.c
@@ -1377,7 +1377,7 @@ convertToJsonb(JsonbValue *val)
 	initStringInfo(&buffer);
 
 	/* Make room for the varlena header */
-	reserveFromBuffer(&buffer, sizeof(VARHDRSZ));
+	reserveFromBuffer(&buffer, VARHDRSZ);
 
 	convertJsonbValue(&buffer, &jentry, val, 0);
 
#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Michael Paquier (#2)
Re: thinko in convertToJsonb()

Michael Paquier <michael.paquier@gmail.com> writes:

On Tue, Dec 9, 2014 at 11:11 AM, Mark Dilger <mark@port25.com> wrote:

Perhaps the code really meant to say:
reserveFromBuffer(&buffer, VARHDRSZ)

Good catch! The code is indeed incorrect. Attached is a one-line patch
addressing that, I guess someone is going to pick up that sooner or
later.

Pushed, thanks!

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers