提交 4f92dd79 编写于 作者: J Jesse Zhang

Use StringInfo when logging distributed snapshot info

Under GCC 8, I get a warning (and rumor has it that you get this under
GCC 7 with `-Wrestrict`):

```
sharedsnapshot.c: In function ‘LogDistributedSnapshotInfo’:
sharedsnapshot.c:924:11: warning: passing argument 1 to
restrict-qualified parameter aliases with argument 4 [-Wrestrict]
  snprintf(message, MESSAGE_LEN, "%s, In progress array: {",
           ^~~~~~~
     message);
     ~~~~~~~
sharedsnapshot.c:930:13: warning: passing argument 1 to
restrict-qualified parameter aliases with argument 4 [-Wrestrict]
    snprintf(message, MESSAGE_LEN, "%s, (dx%d)",
             ^~~~~~~
       message, ds->inProgressXidArray[no]);
       ~~~~~~~
sharedsnapshot.c:933:13: warning: passing argument 1 to
restrict-qualified parameter aliases with argument 4 [-Wrestrict]
    snprintf(message, MESSAGE_LEN, "%s (dx%d)",
             ^~~~~~~
       message, ds->inProgressXidArray[no]);
       ~~~~~~~
```

Upon further inspection, the compiler is right: according to C99, it is
undefined behavior to pass aliased arguments to the "str" argument of
`snprint` (`restrict`-qualified function parameters, to be pedantic).

To make this safer and more readable, this patch switches to using the
StringInfo API. This change might come with a teeny tiny bit of
performance because of:

1. stack vs heap allocation
2. larger initial allocation size of StringInfo

But this area of the code is *never* a hot spot, and `appendStringInfo`
and friends are arguably faster than our old call patterns of
`snprintf`, so I won't sweat on that.

(cherry picked from commit 89553ad2)
(Back port of greenplum-db/gpdb#5753)
上级 be7f190d
......@@ -896,18 +896,18 @@ AtEOXact_SharedSnapshot(void)
void
LogDistributedSnapshotInfo(Snapshot snapshot, const char *prefix)
{
static const int MESSAGE_LEN = 500;
if (!IsMVCCSnapshot(snapshot))
return;
StringInfoData buf;
initStringInfo(&buf);
DistributedSnapshotWithLocalMapping *mapping =
&(snapshot->distribSnapshotWithLocalMapping);
DistributedSnapshot *ds = &mapping->ds;
char message[MESSAGE_LEN];
snprintf(message, MESSAGE_LEN, "%s Distributed snapshot info: "
appendStringInfo(&buf, "%s Distributed snapshot info: "
"xminAllDistributedSnapshots=%d, distribSnapshotId=%d"
", xmin=%d, xmax=%d, count=%d",
prefix,
......@@ -917,18 +917,23 @@ LogDistributedSnapshotInfo(Snapshot snapshot, const char *prefix)
ds->xmax,
ds->count);
snprintf(message, MESSAGE_LEN, "%s, In progress array: {",
message);
appendStringInfoString(&buf, ", In progress array: {");
for (int no = 0; no < ds->count; no++)
{
if (no != 0)
snprintf(message, MESSAGE_LEN, "%s, (dx%d)",
message, ds->inProgressXidArray[no]);
{
appendStringInfo(&buf, ", (dx%d)",
ds->inProgressXidArray[no]);
}
else
snprintf(message, MESSAGE_LEN, "%s (dx%d)",
message, ds->inProgressXidArray[no]);
{
appendStringInfo(&buf, " (dx%d)",
ds->inProgressXidArray[no]);
}
}
appendStringInfoString(&buf, "}");
elog(LOG, "%s}", message);
elog(LOG, "%s", buf.data);
pfree(buf.data);
}
Markdown is supported
0% .
You are about to add 0 people to the discussion. Proceed with caution.
先完成此消息的编辑!
想要评论请 注册