more psprintf() use
Here is a more or less straightforward patch to add more use of
psprintf() in place of hardcoded palloc(N) + sprintf() and the like.
Attachments:
0001-Add-more-use-of-psprintf.patchtext/x-patch; charset=UTF-8; name=0001-Add-more-use-of-psprintf.patchDownload
>From 4a476ed7a37222d87fed068972c88ed884cf25c3 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Wed, 1 Jan 2014 22:09:21 -0500
Subject: [PATCH] Add more use of psprintf()
---
contrib/dblink/dblink.c | 5 +--
contrib/hstore/hstore_io.c | 6 +--
contrib/pageinspect/btreefuncs.c | 73 +++++++++++-----------------------
contrib/pgstattuple/pgstatindex.c | 34 ++++++----------
doc/src/sgml/xtypes.sgml | 3 +-
src/backend/access/transam/multixact.c | 3 +-
src/backend/commands/define.c | 7 +---
src/backend/optimizer/plan/subselect.c | 8 +---
src/backend/parser/parse_type.c | 3 +-
src/backend/storage/smgr/md.c | 4 +-
src/backend/utils/adt/date.c | 6 +--
src/backend/utils/adt/timestamp.c | 7 +---
src/backend/utils/fmgr/funcapi.c | 3 +-
src/test/regress/regress.c | 6 +--
src/tutorial/complex.c | 3 +-
15 files changed, 52 insertions(+), 119 deletions(-)
diff --git a/contrib/dblink/dblink.c b/contrib/dblink/dblink.c
index a91a547..5fd1dd8 100644
--- a/contrib/dblink/dblink.c
+++ b/contrib/dblink/dblink.c
@@ -1563,10 +1563,7 @@ static bool is_valid_dblink_option(const PQconninfoOption *options,
Datum result;
values = (char **) palloc(2 * sizeof(char *));
- values[0] = (char *) palloc(12); /* sign, 10 digits, '\0' */
-
- sprintf(values[0], "%d", call_cntr + 1);
-
+ values[0] = psprintf("%d", call_cntr + 1);
values[1] = results[call_cntr];
/* build the tuple */
diff --git a/contrib/hstore/hstore_io.c b/contrib/hstore/hstore_io.c
index 772a5ca..8331a56 100644
--- a/contrib/hstore/hstore_io.c
+++ b/contrib/hstore/hstore_io.c
@@ -1114,11 +1114,7 @@
HEntry *entries = ARRPTR(in);
if (count == 0)
- {
- out = palloc(1);
- *out = '\0';
- PG_RETURN_CSTRING(out);
- }
+ PG_RETURN_CSTRING("");
buflen = 0;
diff --git a/contrib/pageinspect/btreefuncs.c b/contrib/pageinspect/btreefuncs.c
index bc34af9..e3f3c28 100644
--- a/contrib/pageinspect/btreefuncs.c
+++ b/contrib/pageinspect/btreefuncs.c
@@ -220,31 +220,17 @@
elog(ERROR, "return type must be a row type");
j = 0;
- values[j] = palloc(32);
- snprintf(values[j++], 32, "%d", stat.blkno);
- values[j] = palloc(32);
- snprintf(values[j++], 32, "%c", stat.type);
- values[j] = palloc(32);
- snprintf(values[j++], 32, "%d", stat.live_items);
- values[j] = palloc(32);
- snprintf(values[j++], 32, "%d", stat.dead_items);
- values[j] = palloc(32);
- snprintf(values[j++], 32, "%d", stat.avg_item_size);
- values[j] = palloc(32);
- snprintf(values[j++], 32, "%d", stat.page_size);
- values[j] = palloc(32);
- snprintf(values[j++], 32, "%d", stat.free_size);
- values[j] = palloc(32);
- snprintf(values[j++], 32, "%d", stat.btpo_prev);
- values[j] = palloc(32);
- snprintf(values[j++], 32, "%d", stat.btpo_next);
- values[j] = palloc(32);
- if (stat.type == 'd')
- snprintf(values[j++], 32, "%d", stat.btpo.xact);
- else
- snprintf(values[j++], 32, "%d", stat.btpo.level);
- values[j] = palloc(32);
- snprintf(values[j++], 32, "%d", stat.btpo_flags);
+ values[j++] = psprintf("%d", stat.blkno);
+ values[j++] = psprintf("%c", stat.type);
+ values[j++] = psprintf("%d", stat.live_items);
+ values[j++] = psprintf("%d", stat.dead_items);
+ values[j++] = psprintf("%d", stat.avg_item_size);
+ values[j++] = psprintf("%d", stat.page_size);
+ values[j++] = psprintf("%d", stat.free_size);
+ values[j++] = psprintf("%d", stat.btpo_prev);
+ values[j++] = psprintf("%d", stat.btpo_next);
+ values[j++] = psprintf("%d", (stat.type == 'd') ? stat.btpo.xact : stat.btpo.level);
+ values[j++] = psprintf("%d", stat.btpo_flags);
tuple = BuildTupleFromCStrings(TupleDescGetAttInMetadata(tupleDesc),
values);
@@ -380,18 +366,13 @@ struct user_args
itup = (IndexTuple) PageGetItem(uargs->page, id);
j = 0;
- values[j] = palloc(32);
- snprintf(values[j++], 32, "%d", uargs->offset);
- values[j] = palloc(32);
- snprintf(values[j++], 32, "(%u,%u)",
- BlockIdGetBlockNumber(&(itup->t_tid.ip_blkid)),
- itup->t_tid.ip_posid);
- values[j] = palloc(32);
- snprintf(values[j++], 32, "%d", (int) IndexTupleSize(itup));
- values[j] = palloc(32);
- snprintf(values[j++], 32, "%c", IndexTupleHasNulls(itup) ? 't' : 'f');
- values[j] = palloc(32);
- snprintf(values[j++], 32, "%c", IndexTupleHasVarwidths(itup) ? 't' : 'f');
+ values[j++] = psprintf("%d", uargs->offset);
+ values[j++] = psprintf("(%u,%u)",
+ BlockIdGetBlockNumber(&(itup->t_tid.ip_blkid)),
+ itup->t_tid.ip_posid);
+ values[j++] = psprintf("%d", (int) IndexTupleSize(itup));
+ values[j++] = psprintf("%c", IndexTupleHasNulls(itup) ? 't' : 'f');
+ values[j++] = psprintf("%c", IndexTupleHasVarwidths(itup) ? 't' : 'f');
ptr = (char *) itup + IndexInfoFindDataOffset(itup->t_info);
dlen = IndexTupleSize(itup) - IndexInfoFindDataOffset(itup->t_info);
@@ -477,18 +458,12 @@ struct user_args
elog(ERROR, "return type must be a row type");
j = 0;
- values[j] = palloc(32);
- snprintf(values[j++], 32, "%d", metad->btm_magic);
- values[j] = palloc(32);
- snprintf(values[j++], 32, "%d", metad->btm_version);
- values[j] = palloc(32);
- snprintf(values[j++], 32, "%d", metad->btm_root);
- values[j] = palloc(32);
- snprintf(values[j++], 32, "%d", metad->btm_level);
- values[j] = palloc(32);
- snprintf(values[j++], 32, "%d", metad->btm_fastroot);
- values[j] = palloc(32);
- snprintf(values[j++], 32, "%d", metad->btm_fastlevel);
+ values[j++] = psprintf("%d", metad->btm_magic);
+ values[j++] = psprintf("%d", metad->btm_version);
+ values[j++] = psprintf("%d", metad->btm_root);
+ values[j++] = psprintf("%d", metad->btm_level);
+ values[j++] = psprintf("%d", metad->btm_fastroot);
+ values[j++] = psprintf("%d", metad->btm_fastlevel);
tuple = BuildTupleFromCStrings(TupleDescGetAttInMetadata(tupleDesc),
values);
diff --git a/contrib/pgstattuple/pgstatindex.c b/contrib/pgstattuple/pgstatindex.c
index 282d82c..8939b78 100644
--- a/contrib/pgstattuple/pgstatindex.c
+++ b/contrib/pgstattuple/pgstatindex.c
@@ -271,39 +271,29 @@
elog(ERROR, "return type must be a row type");
j = 0;
- values[j] = palloc(32);
- snprintf(values[j++], 32, "%d", indexStat.version);
- values[j] = palloc(32);
- snprintf(values[j++], 32, "%d", indexStat.level);
- values[j] = palloc(32);
- snprintf(values[j++], 32, INT64_FORMAT,
+ values[j++] = psprintf("%d", indexStat.version);
+ values[j++] = psprintf("%d", indexStat.level);
+ values[j++] = psprintf(INT64_FORMAT,
(indexStat.root_pages +
indexStat.leaf_pages +
indexStat.internal_pages +
indexStat.deleted_pages +
indexStat.empty_pages) * BLCKSZ);
- values[j] = palloc(32);
- snprintf(values[j++], 32, "%u", indexStat.root_blkno);
- values[j] = palloc(32);
- snprintf(values[j++], 32, INT64_FORMAT, indexStat.internal_pages);
- values[j] = palloc(32);
- snprintf(values[j++], 32, INT64_FORMAT, indexStat.leaf_pages);
- values[j] = palloc(32);
- snprintf(values[j++], 32, INT64_FORMAT, indexStat.empty_pages);
- values[j] = palloc(32);
- snprintf(values[j++], 32, INT64_FORMAT, indexStat.deleted_pages);
- values[j] = palloc(32);
+ values[j++] = psprintf("%u", indexStat.root_blkno);
+ values[j++] = psprintf(INT64_FORMAT, indexStat.internal_pages);
+ values[j++] = psprintf(INT64_FORMAT, indexStat.leaf_pages);
+ values[j++] = psprintf(INT64_FORMAT, indexStat.empty_pages);
+ values[j++] = psprintf(INT64_FORMAT, indexStat.deleted_pages);
if (indexStat.max_avail > 0)
- snprintf(values[j++], 32, "%.2f",
+ values[j++] = psprintf("%.2f",
100.0 - (double) indexStat.free_space / (double) indexStat.max_avail * 100.0);
else
- snprintf(values[j++], 32, "NaN");
- values[j] = palloc(32);
+ values[j++] = pstrdup("NaN");
if (indexStat.leaf_pages > 0)
- snprintf(values[j++], 32, "%.2f",
+ values[j++] = psprintf("%.2f",
(double) indexStat.fragments / (double) indexStat.leaf_pages * 100.0);
else
- snprintf(values[j++], 32, "NaN");
+ values[j++] = pstrdup("NaN");
tuple = BuildTupleFromCStrings(TupleDescGetAttInMetadata(tupleDesc),
values);
diff --git a/doc/src/sgml/xtypes.sgml b/doc/src/sgml/xtypes.sgml
index 0154181..e1340ba 100644
--- a/doc/src/sgml/xtypes.sgml
+++ b/doc/src/sgml/xtypes.sgml
@@ -108,8 +108,7 @@ <title>User-defined Types</title>
Complex *complex = (Complex *) PG_GETARG_POINTER(0);
char *result;
- result = (char *) palloc(100);
- snprintf(result, 100, "(%g,%g)", complex->x, complex->y);
+ result = psprintf("(%g,%g)", complex->x, complex->y);
PG_RETURN_CSTRING(result);
}
]]>
diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c
index 55a8ca7..b06aae2 100644
--- a/src/backend/access/transam/multixact.c
+++ b/src/backend/access/transam/multixact.c
@@ -2630,8 +2630,7 @@ static bool MultiXactOffsetPrecedes(MultiXactOffset offset1,
HeapTuple tuple;
char *values[2];
- values[0] = palloc(32);
- sprintf(values[0], "%u", multi->members[multi->iter].xid);
+ values[0] = psprintf("%u", multi->members[multi->iter].xid);
values[1] = mxstatus_to_string(multi->members[multi->iter].status);
tuple = BuildTupleFromCStrings(funccxt->attinmeta, values);
diff --git a/src/backend/commands/define.c b/src/backend/commands/define.c
index 75f77da..486ef92 100644
--- a/src/backend/commands/define.c
+++ b/src/backend/commands/define.c
@@ -56,12 +56,7 @@
switch (nodeTag(def->arg))
{
case T_Integer:
- {
- char *str = palloc(32);
-
- snprintf(str, 32, "%ld", (long) intVal(def->arg));
- return str;
- }
+ return psprintf("%ld", (long) intVal(def->arg));
case T_Float:
/*
diff --git a/src/backend/optimizer/plan/subselect.c b/src/backend/optimizer/plan/subselect.c
index 0f92118..70fdb14 100644
--- a/src/backend/optimizer/plan/subselect.c
+++ b/src/backend/optimizer/plan/subselect.c
@@ -778,10 +778,7 @@ static Bitmapset *finalize_plan(PlannerInfo *root,
sprintf(splan->plan_name + offset, ")");
}
else
- {
- splan->plan_name = palloc(32);
- sprintf(splan->plan_name, "SubPlan %d", splan->plan_id);
- }
+ splan->plan_name = psprintf("SubPlan %d", splan->plan_id);
/* Lastly, fill in the cost estimates for use later */
cost_subplan(root, splan, plan);
@@ -2600,8 +2597,7 @@ static Bitmapset *finalize_plan(PlannerInfo *root,
node->setParam = list_make1_int(prm->paramid);
/* Label the subplan for EXPLAIN purposes */
- node->plan_name = palloc(64);
- sprintf(node->plan_name, "InitPlan %d (returns $%d)",
+ node->plan_name = psprintf("InitPlan %d (returns $%d)",
node->plan_id, prm->paramid);
return prm;
diff --git a/src/backend/parser/parse_type.c b/src/backend/parser/parse_type.c
index ee6802a..84ec8a7 100644
--- a/src/backend/parser/parse_type.c
+++ b/src/backend/parser/parse_type.c
@@ -313,8 +313,7 @@ static int32 typenameTypeMod(ParseState *pstate, const TypeName *typeName,
if (IsA(&ac->val, Integer))
{
- cstr = (char *) palloc(32);
- snprintf(cstr, 32, "%ld", (long) ac->val.val.ival);
+ cstr = psprintf("%ld", (long) ac->val.val.ival);
}
else if (IsA(&ac->val, Float) ||
IsA(&ac->val, String))
diff --git a/src/backend/storage/smgr/md.c b/src/backend/storage/smgr/md.c
index e629181..268035c 100644
--- a/src/backend/storage/smgr/md.c
+++ b/src/backend/storage/smgr/md.c
@@ -1649,9 +1649,7 @@ static BlockNumber _mdnblocks(SMgrRelation reln, ForkNumber forknum,
if (segno > 0)
{
- /* be sure we have enough space for the '.segno' */
- fullpath = (char *) palloc(strlen(path) + 12);
- sprintf(fullpath, "%s.%u", path, segno);
+ fullpath = psprintf("%s.%u", path, segno);
pfree(path);
}
else
diff --git a/src/backend/utils/adt/date.c b/src/backend/utils/adt/date.c
index fe091da..2bafd61 100644
--- a/src/backend/utils/adt/date.c
+++ b/src/backend/utils/adt/date.c
@@ -91,14 +91,12 @@
static char *
anytime_typmodout(bool istz, int32 typmod)
{
- char *res = (char *) palloc(64);
const char *tz = istz ? " with time zone" : " without time zone";
if (typmod >= 0)
- snprintf(res, 64, "(%d)%s", (int) typmod, tz);
+ return psprintf("(%d)%s", (int) typmod, tz);
else
- snprintf(res, 64, "%s", tz);
- return res;
+ return psprintf("%s", tz);
}
diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c
index c3c71b7..5263d80 100644
--- a/src/backend/utils/adt/timestamp.c
+++ b/src/backend/utils/adt/timestamp.c
@@ -116,15 +116,12 @@
static char *
anytimestamp_typmodout(bool istz, int32 typmod)
{
- char *res = (char *) palloc(64);
const char *tz = istz ? " with time zone" : " without time zone";
if (typmod >= 0)
- snprintf(res, 64, "(%d)%s", (int) typmod, tz);
+ return psprintf("(%d)%s", (int) typmod, tz);
else
- snprintf(res, 64, "%s", tz);
-
- return res;
+ return psprintf("%s", tz);
}
diff --git a/src/backend/utils/fmgr/funcapi.c b/src/backend/utils/fmgr/funcapi.c
index 6347a8f..7718f43 100644
--- a/src/backend/utils/fmgr/funcapi.c
+++ b/src/backend/utils/fmgr/funcapi.c
@@ -1205,8 +1205,7 @@ static bool resolve_polymorphic_tupdesc(TupleDesc tupdesc,
if (pname == NULL || pname[0] == '\0')
{
/* Parameter is not named, so gin up a column name */
- pname = (char *) palloc(32);
- snprintf(pname, 32, "column%d", numoutargs + 1);
+ pname = psprintf("column%d", numoutargs + 1);
}
outargnames[numoutargs] = pname;
numoutargs++;
diff --git a/src/test/regress/regress.c b/src/test/regress/regress.c
index 3bd8a15..4dbe314 100644
--- a/src/test/regress/regress.c
+++ b/src/test/regress/regress.c
@@ -269,15 +269,11 @@
char *
widget_out(WIDGET * widget)
{
- char *result;
-
if (widget == NULL)
return NULL;
- result = (char *) palloc(60);
- sprintf(result, "(%g,%g,%g)",
+ return psprintf("(%g,%g,%g)",
widget->center.x, widget->center.y, widget->radius);
- return result;
}
PG_FUNCTION_INFO_V1(pt_in_widget);
diff --git a/src/tutorial/complex.c b/src/tutorial/complex.c
index edae006..8065588 100644
--- a/src/tutorial/complex.c
+++ b/src/tutorial/complex.c
@@ -73,8 +73,7 @@
Complex *complex = (Complex *) PG_GETARG_POINTER(0);
char *result;
- result = (char *) palloc(100);
- snprintf(result, 100, "(%g,%g)", complex->x, complex->y);
+ result = psprintf("(%g,%g)", complex->x, complex->y);
PG_RETURN_CSTRING(result);
}
--
1.8.5.1
On Wed, Jan 1, 2014 at 10:14 PM, Peter Eisentraut <peter_e@gmx.net> wrote:
Here is a more or less straightforward patch to add more use of
psprintf() in place of hardcoded palloc(N) + sprintf() and the like.
Looks nifty.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 01/02/2014 05:14 AM, Peter Eisentraut wrote:
diff --git a/contrib/hstore/hstore_io.c b/contrib/hstore/hstore_io.c index 772a5ca..8331a56 100644 --- a/contrib/hstore/hstore_io.c +++ b/contrib/hstore/hstore_io.c @@ -1114,11 +1114,7 @@ HEntry *entries = ARRPTR(in);if (count == 0) - { - out = palloc(1); - *out = '\0'; - PG_RETURN_CSTRING(out); - } + PG_RETURN_CSTRING("");buflen = 0;
Is it legal to return a constant with PG_RETURN_CSTRING? Grepping
around, I don't see that being done anywhere else, but there are places
that do PG_RETURN_CSTRING(pstrdup(<constant>))...
- Heikki
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2014-01-02 09:49:48 +0200, Heikki Linnakangas wrote:
On 01/02/2014 05:14 AM, Peter Eisentraut wrote:
diff --git a/contrib/hstore/hstore_io.c b/contrib/hstore/hstore_io.c index 772a5ca..8331a56 100644 --- a/contrib/hstore/hstore_io.c +++ b/contrib/hstore/hstore_io.c @@ -1114,11 +1114,7 @@ HEntry *entries = ARRPTR(in);if (count == 0) - { - out = palloc(1); - *out = '\0'; - PG_RETURN_CSTRING(out); - } + PG_RETURN_CSTRING("");buflen = 0;
Is it legal to return a constant with PG_RETURN_CSTRING? Grepping around, I
don't see that being done anywhere else, but there are places that do
PG_RETURN_CSTRING(pstrdup(<constant>))...
I don't see why it wouldn't be legal - constant strings have static
storage duration, i.e. the program lifetime. And I can see nothing that
would allow pfree()ing the return value of cstring returning functions
in the general case.
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Andres Freund <andres@2ndquadrant.com> writes:
On 2014-01-02 09:49:48 +0200, Heikki Linnakangas wrote:
Is it legal to return a constant with PG_RETURN_CSTRING? Grepping around, I
don't see that being done anywhere else, but there are places that do
PG_RETURN_CSTRING(pstrdup(<constant>))...
I don't see why it wouldn't be legal - constant strings have static
storage duration, i.e. the program lifetime. And I can see nothing that
would allow pfree()ing the return value of cstring returning functions
in the general case.
Heikki is right and you are wrong. There is an ancient supposition that
datatype output functions, in particular, always return palloc'd strings.
I recently got rid of the pfree's in the main output path, cf commit
b006f4ddb988568081f8290fac77f9402b137120, which might explain why this
patch passes regression tests; but there are still places in the code (and
even more likely in third-party code) that will try to pfree the results.
So -1 for this particular change. The pstrdup that Heikki suggests would
be safer practice.
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
Peter Eisentraut wrote:
psprintf() in place of hardcoded palloc(N) + sprintf() and the like.
+ values[j++] = psprintf("%d", stat.blkno); + values[j++] = psprintf("%c", stat.type); + values[j++] = psprintf("%d", stat.live_items); + values[j++] = psprintf("%d", stat.dead_items); + values[j++] = psprintf("%d", stat.avg_item_size); + values[j++] = psprintf("%d", stat.page_size); + values[j++] = psprintf("%d", stat.free_size); + values[j++] = psprintf("%d", stat.btpo_prev); + values[j++] = psprintf("%d", stat.btpo_next); + values[j++] = psprintf("%d", (stat.type == 'd') ? stat.btpo.xact : stat.btpo.level); + values[j++] = psprintf("%d", stat.btpo_flags);tuple = BuildTupleFromCStrings(TupleDescGetAttInMetadata(tupleDesc),
values);
In cases such as this one, I have often wondered whether it'd be better
to write this as DatumGetSometype() plus heap_form_tuple, instead of
printing to strings and then building a tuple from those.
--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 1/2/14, 9:28 AM, Tom Lane wrote:
Heikki is right and you are wrong. There is an ancient supposition that
datatype output functions, in particular, always return palloc'd strings.I recently got rid of the pfree's in the main output path, cf commit
b006f4ddb988568081f8290fac77f9402b137120, which might explain why this
patch passes regression tests; but there are still places in the code (and
even more likely in third-party code) that will try to pfree the results.
Well, that seems kind of dangerous. The next guys is going to write an
extension that is returning string constants directly, and there is no
straightforward way to detect this problem. Perhaps we should have some
mode similar to the CLOBBER and COPY_*_TREES symbols to force a pfree()
in assertion-enabled builds?
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 1/2/14, 2:12 PM, Alvaro Herrera wrote:
Peter Eisentraut wrote:
psprintf() in place of hardcoded palloc(N) + sprintf() and the like.
+ values[j++] = psprintf("%d", stat.blkno); + values[j++] = psprintf("%c", stat.type); + values[j++] = psprintf("%d", stat.live_items); + values[j++] = psprintf("%d", stat.dead_items); + values[j++] = psprintf("%d", stat.avg_item_size); + values[j++] = psprintf("%d", stat.page_size); + values[j++] = psprintf("%d", stat.free_size); + values[j++] = psprintf("%d", stat.btpo_prev); + values[j++] = psprintf("%d", stat.btpo_next); + values[j++] = psprintf("%d", (stat.type == 'd') ? stat.btpo.xact : stat.btpo.level); + values[j++] = psprintf("%d", stat.btpo_flags);tuple = BuildTupleFromCStrings(TupleDescGetAttInMetadata(tupleDesc),
values);In cases such as this one, I have often wondered whether it'd be better
to write this as DatumGetSometype() plus heap_form_tuple, instead of
printing to strings and then building a tuple from those.
Probably. As you can see, this style is only used in a few contrib
modules that all came from the same source, I think.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Peter Eisentraut <peter_e@gmx.net> writes:
On 1/2/14, 9:28 AM, Tom Lane wrote:
Heikki is right and you are wrong. There is an ancient supposition that
datatype output functions, in particular, always return palloc'd strings.I recently got rid of the pfree's in the main output path, cf commit
b006f4ddb988568081f8290fac77f9402b137120, which might explain why this
patch passes regression tests; but there are still places in the code (and
even more likely in third-party code) that will try to pfree the results.
Well, that seems kind of dangerous. The next guys is going to write an
extension that is returning string constants directly, and there is no
straightforward way to detect this problem. Perhaps we should have some
mode similar to the CLOBBER and COPY_*_TREES symbols to force a pfree()
in assertion-enabled builds?
Seems kinda backwards. If we want to put any effort into this issue,
it'd be better to head in the direction of making the world safe for
output functions to return constants, ie deprecate rather than enforce
the practice of pfree'ing their results. But see
/messages/by-id/12646.1383420576@sss.pgh.pa.us
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