Skip to content

Commit d7112cf

Browse files
tglsfdctglsfdc
committed
Remove the last vestiges of the MAKE_PTR/MAKE_OFFSET mechanism. We haven't
allowed different processes to have different addresses for the shmem segment in quite a long time, but there were still a few places left that used the old coding convention. Clean them up to reduce confusion and improve the compiler's ability to detect pointer type mismatches. Kris Jurka
1 parent 902d1cb commit d7112cf

File tree

10 files changed

+153
-268
lines changed

10 files changed

+153
-268
lines changed

‎src/backend/access/transam/twophase.c

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* Portions Copyright (c) 1994, Regents of the University of California
88
*
99
* IDENTIFICATION
10-
* $PostgreSQL: pgsql/src/backend/access/transam/twophase.c,v 1.46 2008/10/20 19:18:18 alvherre Exp $
10+
* $PostgreSQL: pgsql/src/backend/access/transam/twophase.c,v 1.47 2008/11/02 21:24:51 tgl Exp $
1111
*
1212
* NOTES
1313
* Each global transaction is associated with a global transaction
@@ -122,7 +122,7 @@ typedef struct GlobalTransactionData
122122
typedef struct TwoPhaseStateData
123123
{
124124
/* Head of linked list of free GlobalTransactionData structs */
125-
SHMEM_OFFSET freeGXacts;
125+
GlobalTransaction freeGXacts;
126126

127127
/* Number of valid prepXacts entries. */
128128
int numPrepXacts;
@@ -184,7 +184,7 @@ TwoPhaseShmemInit(void)
184184
int i;
185185

186186
Assert(!found);
187-
TwoPhaseState->freeGXacts = INVALID_OFFSET;
187+
TwoPhaseState->freeGXacts = NULL;
188188
TwoPhaseState->numPrepXacts = 0;
189189

190190
/*
@@ -196,8 +196,8 @@ TwoPhaseShmemInit(void)
196196
sizeof(GlobalTransaction) * max_prepared_xacts));
197197
for (i = 0; i < max_prepared_xacts; i++)
198198
{
199-
gxacts[i].proc.links.next = TwoPhaseState->freeGXacts;
200-
TwoPhaseState->freeGXacts = MAKE_OFFSET(&gxacts[i]);
199+
gxacts[i].proc.links.next = (SHM_QUEUE *) TwoPhaseState->freeGXacts;
200+
TwoPhaseState->freeGXacts = &gxacts[i];
201201
}
202202
}
203203
else
@@ -242,8 +242,8 @@ MarkAsPreparing(TransactionId xid, const char *gid,
242242
TwoPhaseState->numPrepXacts--;
243243
TwoPhaseState->prepXacts[i] = TwoPhaseState->prepXacts[TwoPhaseState->numPrepXacts];
244244
/* and put it back in the freelist */
245-
gxact->proc.links.next = TwoPhaseState->freeGXacts;
246-
TwoPhaseState->freeGXacts = MAKE_OFFSET(gxact);
245+
gxact->proc.links.next = (SHM_QUEUE *) TwoPhaseState->freeGXacts;
246+
TwoPhaseState->freeGXacts = gxact;
247247
/* Back up index count too, so we don't miss scanning one */
248248
i--;
249249
}
@@ -263,14 +263,14 @@ MarkAsPreparing(TransactionId xid, const char *gid,
263263
}
264264

265265
/* Get a free gxact from the freelist */
266-
if (TwoPhaseState->freeGXacts == INVALID_OFFSET)
266+
if (TwoPhaseState->freeGXacts == NULL)
267267
ereport(ERROR,
268268
(errcode(ERRCODE_OUT_OF_MEMORY),
269269
errmsg("maximum number of prepared transactions reached"),
270270
errhint("Increase max_prepared_transactions (currently %d).",
271271
max_prepared_xacts)));
272-
gxact = (GlobalTransaction) MAKE_PTR(TwoPhaseState->freeGXacts);
273-
TwoPhaseState->freeGXacts = gxact->proc.links.next;
272+
gxact = TwoPhaseState->freeGXacts;
273+
TwoPhaseState->freeGXacts = (GlobalTransaction) gxact->proc.links.next;
274274

275275
/* Initialize it */
276276
MemSet(&gxact->proc, 0, sizeof(PGPROC));
@@ -451,8 +451,8 @@ RemoveGXact(GlobalTransaction gxact)
451451
TwoPhaseState->prepXacts[i] = TwoPhaseState->prepXacts[TwoPhaseState->numPrepXacts];
452452

453453
/* and put it back in the freelist */
454-
gxact->proc.links.next = TwoPhaseState->freeGXacts;
455-
TwoPhaseState->freeGXacts = MAKE_OFFSET(gxact);
454+
gxact->proc.links.next = (SHM_QUEUE *) TwoPhaseState->freeGXacts;
455+
TwoPhaseState->freeGXacts = gxact;
456456

457457
LWLockRelease(TwoPhaseStateLock);
458458

‎src/backend/postmaster/autovacuum.c

Lines changed: 27 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@
5555
*
5656
*
5757
* IDENTIFICATION
58-
* $PostgreSQL: pgsql/src/backend/postmaster/autovacuum.c,v 1.84 2008/08/13 00:07:50 alvherre Exp $
58+
* $PostgreSQL: pgsql/src/backend/postmaster/autovacuum.c,v 1.85 2008/11/02 21:24:52 tgl Exp $
5959
*
6060
*-------------------------------------------------------------------------
6161
*/
@@ -244,9 +244,9 @@ typedef struct
244244
{
245245
sig_atomic_t av_signal[AutoVacNumSignals];
246246
pid_t av_launcherpid;
247-
SHMEM_OFFSET av_freeWorkers;
247+
WorkerInfo av_freeWorkers;
248248
SHM_QUEUE av_runningWorkers;
249-
SHMEM_OFFSET av_startingWorker;
249+
WorkerInfo av_startingWorker;
250250
} AutoVacuumShmemStruct;
251251

252252
static AutoVacuumShmemStruct *AutoVacuumShmem;
@@ -556,8 +556,8 @@ AutoVacLauncherMain(int argc, char *argv[])
556556
if (!PostmasterIsAlive(true))
557557
exit(1);
558558

559-
launcher_determine_sleep(AutoVacuumShmem->av_freeWorkers !=
560-
INVALID_OFFSET, false, &nap);
559+
launcher_determine_sleep((AutoVacuumShmem->av_freeWorkers != NULL),
560+
false, &nap);
561561

562562
/*
563563
* Sleep for a while according to schedule.
@@ -662,13 +662,12 @@ AutoVacLauncherMain(int argc, char *argv[])
662662
current_time = GetCurrentTimestamp();
663663
LWLockAcquire(AutovacuumLock, LW_SHARED);
664664

665-
can_launch = (AutoVacuumShmem->av_freeWorkers != INVALID_OFFSET);
665+
can_launch = (AutoVacuumShmem->av_freeWorkers != NULL);
666666

667-
if (AutoVacuumShmem->av_startingWorker != INVALID_OFFSET)
667+
if (AutoVacuumShmem->av_startingWorker != NULL)
668668
{
669669
int waittime;
670-
671-
WorkerInfo worker = (WorkerInfo) MAKE_PTR(AutoVacuumShmem->av_startingWorker);
670+
WorkerInfo worker = AutoVacuumShmem->av_startingWorker;
672671

673672
/*
674673
* We can't launch another worker when another one is still
@@ -698,16 +697,16 @@ AutoVacLauncherMain(int argc, char *argv[])
698697
* we assume it's the same one we saw above (so we don't
699698
* recheck the launch time).
700699
*/
701-
if (AutoVacuumShmem->av_startingWorker != INVALID_OFFSET)
700+
if (AutoVacuumShmem->av_startingWorker != NULL)
702701
{
703-
worker = (WorkerInfo) MAKE_PTR(AutoVacuumShmem->av_startingWorker);
702+
worker = AutoVacuumShmem->av_startingWorker;
704703
worker->wi_dboid = InvalidOid;
705704
worker->wi_tableoid = InvalidOid;
706705
worker->wi_proc = NULL;
707706
worker->wi_launchtime = 0;
708-
worker->wi_links.next = AutoVacuumShmem->av_freeWorkers;
709-
AutoVacuumShmem->av_freeWorkers = MAKE_OFFSET(worker);
710-
AutoVacuumShmem->av_startingWorker = INVALID_OFFSET;
707+
worker->wi_links.next = (SHM_QUEUE *) AutoVacuumShmem->av_freeWorkers;
708+
AutoVacuumShmem->av_freeWorkers = worker;
709+
AutoVacuumShmem->av_startingWorker = NULL;
711710
elog(WARNING, "worker took too long to start; cancelled");
712711
}
713712
}
@@ -1061,7 +1060,7 @@ do_start_worker(void)
10611060

10621061
/* return quickly when there are no free workers */
10631062
LWLockAcquire(AutovacuumLock, LW_SHARED);
1064-
if (AutoVacuumShmem->av_freeWorkers == INVALID_OFFSET)
1063+
if (AutoVacuumShmem->av_freeWorkers == NULL)
10651064
{
10661065
LWLockRelease(AutovacuumLock);
10671066
return InvalidOid;
@@ -1192,7 +1191,6 @@ do_start_worker(void)
11921191
if (avdb != NULL)
11931192
{
11941193
WorkerInfo worker;
1195-
SHMEM_OFFSET sworker;
11961194

11971195
LWLockAcquire(AutovacuumLock, LW_EXCLUSIVE);
11981196

@@ -1201,18 +1199,17 @@ do_start_worker(void)
12011199
* really should be a free slot -- complain very loudly if there
12021200
* isn't.
12031201
*/
1204-
sworker = AutoVacuumShmem->av_freeWorkers;
1205-
if (sworker == INVALID_OFFSET)
1202+
worker = AutoVacuumShmem->av_freeWorkers;
1203+
if (worker == NULL)
12061204
elog(FATAL, "no free worker found");
12071205

1208-
worker = (WorkerInfo) MAKE_PTR(sworker);
1209-
AutoVacuumShmem->av_freeWorkers = worker->wi_links.next;
1206+
AutoVacuumShmem->av_freeWorkers = (WorkerInfo) worker->wi_links.next;
12101207

12111208
worker->wi_dboid = avdb->adw_datid;
12121209
worker->wi_proc = NULL;
12131210
worker->wi_launchtime = GetCurrentTimestamp();
12141211

1215-
AutoVacuumShmem->av_startingWorker = sworker;
1212+
AutoVacuumShmem->av_startingWorker = worker;
12161213

12171214
LWLockRelease(AutovacuumLock);
12181215

@@ -1549,9 +1546,9 @@ AutoVacWorkerMain(int argc, char *argv[])
15491546
* launcher might have decided to remove it from the queue and start
15501547
* again.
15511548
*/
1552-
if (AutoVacuumShmem->av_startingWorker != INVALID_OFFSET)
1549+
if (AutoVacuumShmem->av_startingWorker != NULL)
15531550
{
1554-
MyWorkerInfo = (WorkerInfo) MAKE_PTR(AutoVacuumShmem->av_startingWorker);
1551+
MyWorkerInfo = AutoVacuumShmem->av_startingWorker;
15551552
dbid = MyWorkerInfo->wi_dboid;
15561553
MyWorkerInfo->wi_proc = MyProc;
15571554

@@ -1563,7 +1560,7 @@ AutoVacWorkerMain(int argc, char *argv[])
15631560
* remove from the "starting" pointer, so that the launcher can start
15641561
* a new worker if required
15651562
*/
1566-
AutoVacuumShmem->av_startingWorker = INVALID_OFFSET;
1563+
AutoVacuumShmem->av_startingWorker = NULL;
15671564
LWLockRelease(AutovacuumLock);
15681565

15691566
on_shmem_exit(FreeWorkerInfo, 0);
@@ -1648,15 +1645,15 @@ FreeWorkerInfo(int code, Datum arg)
16481645
AutovacuumLauncherPid = AutoVacuumShmem->av_launcherpid;
16491646

16501647
SHMQueueDelete(&MyWorkerInfo->wi_links);
1651-
MyWorkerInfo->wi_links.next = AutoVacuumShmem->av_freeWorkers;
1648+
MyWorkerInfo->wi_links.next = (SHM_QUEUE *) AutoVacuumShmem->av_freeWorkers;
16521649
MyWorkerInfo->wi_dboid = InvalidOid;
16531650
MyWorkerInfo->wi_tableoid = InvalidOid;
16541651
MyWorkerInfo->wi_proc = NULL;
16551652
MyWorkerInfo->wi_launchtime = 0;
16561653
MyWorkerInfo->wi_cost_delay = 0;
16571654
MyWorkerInfo->wi_cost_limit = 0;
16581655
MyWorkerInfo->wi_cost_limit_base = 0;
1659-
AutoVacuumShmem->av_freeWorkers = MAKE_OFFSET(MyWorkerInfo);
1656+
AutoVacuumShmem->av_freeWorkers = MyWorkerInfo;
16601657
/* not mine anymore */
16611658
MyWorkerInfo = NULL;
16621659

@@ -2793,18 +2790,18 @@ AutoVacuumShmemInit(void)
27932790
Assert(!found);
27942791

27952792
AutoVacuumShmem->av_launcherpid = 0;
2796-
AutoVacuumShmem->av_freeWorkers = INVALID_OFFSET;
2793+
AutoVacuumShmem->av_freeWorkers = NULL;
27972794
SHMQueueInit(&AutoVacuumShmem->av_runningWorkers);
2798-
AutoVacuumShmem->av_startingWorker = INVALID_OFFSET;
2795+
AutoVacuumShmem->av_startingWorker = NULL;
27992796

28002797
worker = (WorkerInfo) ((char *) AutoVacuumShmem +
28012798
MAXALIGN(sizeof(AutoVacuumShmemStruct)));
28022799

28032800
/* initialize the WorkerInfo free list */
28042801
for (i = 0; i < autovacuum_max_workers; i++)
28052802
{
2806-
worker[i].wi_links.next = AutoVacuumShmem->av_freeWorkers;
2807-
AutoVacuumShmem->av_freeWorkers = MAKE_OFFSET(&worker[i]);
2803+
worker[i].wi_links.next = (SHM_QUEUE *) AutoVacuumShmem->av_freeWorkers;
2804+
AutoVacuumShmem->av_freeWorkers = &worker[i];
28082805
}
28092806
}
28102807
else

‎src/backend/storage/ipc/shmem.c

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/storage/ipc/shmem.c,v 1.100 2008/01/01 19:45:51 momjian Exp $
11+
* $PostgreSQL: pgsql/src/backend/storage/ipc/shmem.c,v 1.101 2008/11/02 21:24:52 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -77,9 +77,9 @@
7777

7878
static PGShmemHeader *ShmemSegHdr; /* shared mem segment header */
7979

80-
SHMEM_OFFSET ShmemBase; /* start address of shared memory */
80+
static void *ShmemBase; /* start address of shared memory */
8181

82-
static SHMEM_OFFSET ShmemEnd; /* end+1 address of shared memory */
82+
static void *ShmemEnd; /* end+1 address of shared memory */
8383

8484
slock_t *ShmemLock; /* spinlock for shared memory and LWLock
8585
* allocation */
@@ -99,8 +99,8 @@ InitShmemAccess(void *seghdr)
9999
PGShmemHeader *shmhdr = (PGShmemHeader *) seghdr;
100100

101101
ShmemSegHdr = shmhdr;
102-
ShmemBase = (SHMEM_OFFSET) shmhdr;
103-
ShmemEnd = ShmemBase + shmhdr->totalsize;
102+
ShmemBase = (void *) shmhdr;
103+
ShmemEnd = (char *) ShmemBase + shmhdr->totalsize;
104104
}
105105

106106
/*
@@ -127,7 +127,7 @@ InitShmemAllocation(void)
127127
SpinLockInit(ShmemLock);
128128

129129
/* ShmemIndex can't be set up yet (need LWLocks first) */
130-
shmhdr->indexoffset = 0;
130+
shmhdr->index = NULL;
131131
ShmemIndex = (HTAB *) NULL;
132132

133133
/*
@@ -176,7 +176,7 @@ ShmemAlloc(Size size)
176176
newFree = newStart + size;
177177
if (newFree <= shmemseghdr->totalsize)
178178
{
179-
newSpace = (void *) MAKE_PTR(newStart);
179+
newSpace = (void *) ((char *) ShmemBase + newStart);
180180
shmemseghdr->freeoffset = newFree;
181181
}
182182
else
@@ -193,14 +193,14 @@ ShmemAlloc(Size size)
193193
}
194194

195195
/*
196-
* ShmemIsValid -- test if an offset refers to valid shared memory
196+
* ShmemAddrIsValid -- test if an address refers to shared memory
197197
*
198-
* Returns TRUE if the pointer is valid.
198+
* Returns TRUE if the pointer points within the shared memory segment.
199199
*/
200200
bool
201-
ShmemIsValid(unsigned long addr)
201+
ShmemAddrIsValid(void *addr)
202202
{
203-
return (addr < ShmemEnd) && (addr >= ShmemBase);
203+
return (addr >= ShmemBase) && (addr < ShmemEnd);
204204
}
205205

206206
/*
@@ -324,8 +324,8 @@ ShmemInitStruct(const char *name, Size size, bool *foundPtr)
324324
if (IsUnderPostmaster)
325325
{
326326
/* Must be initializing a (non-standalone) backend */
327-
Assert(shmemseghdr->indexoffset != 0);
328-
structPtr = (void *) MAKE_PTR(shmemseghdr->indexoffset);
327+
Assert(shmemseghdr->index != NULL);
328+
structPtr = shmemseghdr->index;
329329
*foundPtr = TRUE;
330330
}
331331
else
@@ -338,9 +338,9 @@ ShmemInitStruct(const char *name, Size size, bool *foundPtr)
338338
* index has been initialized. This should be OK because no other
339339
* process can be accessing shared memory yet.
340340
*/
341-
Assert(shmemseghdr->indexoffset == 0);
341+
Assert(shmemseghdr->index == NULL);
342342
structPtr = ShmemAlloc(size);
343-
shmemseghdr->indexoffset = MAKE_OFFSET(structPtr);
343+
shmemseghdr->index = structPtr;
344344
*foundPtr = FALSE;
345345
}
346346
LWLockRelease(ShmemIndexLock);
@@ -374,7 +374,7 @@ ShmemInitStruct(const char *name, Size size, bool *foundPtr)
374374
/* let caller print its message too */
375375
return NULL;
376376
}
377-
structPtr = (void *) MAKE_PTR(result->location);
377+
structPtr = result->location;
378378
}
379379
else
380380
{
@@ -395,9 +395,9 @@ ShmemInitStruct(const char *name, Size size, bool *foundPtr)
395395
return NULL;
396396
}
397397
result->size = size;
398-
result->location = MAKE_OFFSET(structPtr);
398+
result->location = structPtr;
399399
}
400-
Assert(ShmemIsValid((unsigned long) structPtr));
400+
Assert(ShmemAddrIsValid(structPtr));
401401

402402
LWLockRelease(ShmemIndexLock);
403403
return structPtr;

0 commit comments

Comments
 (0)