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 (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to