Hello
Currently there is a following piece of code in snapmgr.c:
```
/* Copy all required fields */
snapshot = (Snapshot) MemoryContextAlloc(TopTransactionContext, size);
snapshot->satisfies = HeapTupleSatisfiesMVCC;
snapshot->xmin = serialized_snapshot->xmin;
snapshot->xmax = serialized_snapshot->xmax;
snapshot->xip = NULL;
snapshot->xcnt = serialized_snapshot->xcnt;
snapshot->subxip = NULL;
/* ... */
/* Copy XIDs, if present. */
if (serialized_snapshot->xcnt > 0)
{
snapshot->xip = (TransactionId *) (snapshot + 1);
memcpy(snapshot->xip, serialized_xids,
serialized_snapshot->xcnt * sizeof(TransactionId));
}
/* Copy SubXIDs, if present. */
if (serialized_snapshot->subxcnt > 0)
{
snapshot->subxip = snapshot->xip + serialized_snapshot->xcnt;
memcpy(snapshot->subxip, ...
```
It assumes that subxcnt > 0 iff xcnt > 0. As I understand this is true.
At least I hope so, otherwise this code can call memcpy passing NULL as
a first argument. But Clang Static Analyzer is not aware of all this:
http://afiskon.ru/s/db/5c956077e9_snapmgr.png
I propose a patch that makes static analyzers happy and makes intention
of this code a little more obvious.
--
Best regards,
Aleksander Alekseev
http://eax.me/
diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c
index b88e012..329bbba 100644
--- a/src/backend/utils/time/snapmgr.c
+++ b/src/backend/utils/time/snapmgr.c
@@ -1538,7 +1538,8 @@ RestoreSnapshot(char *start_address)
SerializedSnapshotData *serialized_snapshot;
Size size;
Snapshot snapshot;
- TransactionId *serialized_xids;
+ TransactionId *serialized_xids,
+ *xids_start;
serialized_snapshot = (SerializedSnapshotData *) start_address;
serialized_xids = (TransactionId *)
@@ -1562,10 +1563,12 @@ RestoreSnapshot(char *start_address)
snapshot->takenDuringRecovery = serialized_snapshot->takenDuringRecovery;
snapshot->curcid = serialized_snapshot->curcid;
+ xids_start = (TransactionId *) (snapshot + 1);
+
/* Copy XIDs, if present. */
if (serialized_snapshot->xcnt > 0)
{
- snapshot->xip = (TransactionId *) (snapshot + 1);
+ snapshot->xip = xids_start;
memcpy(snapshot->xip, serialized_xids,
serialized_snapshot->xcnt * sizeof(TransactionId));
}
@@ -1573,7 +1576,7 @@ RestoreSnapshot(char *start_address)
/* Copy SubXIDs, if present. */
if (serialized_snapshot->subxcnt > 0)
{
- snapshot->subxip = snapshot->xip + serialized_snapshot->xcnt;
+ snapshot->subxip = xids_start + serialized_snapshot->xcnt;
memcpy(snapshot->subxip, serialized_xids + serialized_snapshot->xcnt,
serialized_snapshot->subxcnt * sizeof(TransactionId));
}
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
