Skip to content

Commit e378f82

Browse files
tglsfdctglsfdc
committed
Make use of qsort_arg in several places that were formerly using klugy
static variables. This avoids any risk of potential non-reentrancy, and in particular offers a much cleaner workaround for the Intel compiler bug that was affecting ginutil.c.
1 parent f2332ea commit e378f82

File tree

6 files changed

+77
-88
lines changed

6 files changed

+77
-88
lines changed

‎contrib/btree_gist/btree_utils_var.c

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -439,18 +439,14 @@ gbt_var_penalty(float *res, const GISTENTRY *o, const GISTENTRY *n, const gbtree
439439
}
440440

441441

442-
/*
443-
* Fortunately, this sort comparison routine needn't be reentrant...
444-
*/
445-
static const gbtree_vinfo *gbt_vsrt_cmp_tinfo;
446-
447442
static int
448-
gbt_vsrt_cmp(const void *a, const void *b)
443+
gbt_vsrt_cmp(const void *a, const void *b, void *arg)
449444
{
450445
GBT_VARKEY_R ar = gbt_var_key_readable(((const Vsrt *) a)->t);
451446
GBT_VARKEY_R br = gbt_var_key_readable(((const Vsrt *) b)->t);
447+
const gbtree_vinfo *tinfo = (const gbtree_vinfo *) arg;
452448

453-
return (*gbt_vsrt_cmp_tinfo->f_cmp) (ar.lower, br.lower);
449+
return (*tinfo->f_cmp) (ar.lower, br.lower);
454450
}
455451

456452
GIST_SPLITVEC *
@@ -496,11 +492,11 @@ gbt_var_picksplit(const GistEntryVector *entryvec, GIST_SPLITVEC *v, const gbtre
496492
}
497493

498494
/* sort */
499-
gbt_vsrt_cmp_tinfo = tinfo;
500-
qsort((void *) &arr[FirstOffsetNumber],
501-
maxoff - FirstOffsetNumber + 1,
502-
sizeof(Vsrt),
503-
gbt_vsrt_cmp);
495+
qsort_arg((void *) &arr[FirstOffsetNumber],
496+
maxoff - FirstOffsetNumber + 1,
497+
sizeof(Vsrt),
498+
gbt_vsrt_cmp,
499+
(void *) tinfo);
504500

505501
/* We do simply create two parts */
506502

‎contrib/tsearch2/rank.c

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -122,14 +122,14 @@ find_wordentry(tsvector * t, QUERYTYPE * q, ITEM * item)
122122
}
123123

124124

125-
static char *SortAndUniqOperand = NULL;
126-
127125
static int
128-
compareITEM(const void *a, const void *b)
126+
compareITEM(const void *a, const void *b, void *arg)
129127
{
128+
char *operand = (char *) arg;
129+
130130
if ((*(ITEM **) a)->length == (*(ITEM **) b)->length)
131-
return strncmp(SortAndUniqOperand + (*(ITEM **) a)->distance,
132-
SortAndUniqOperand + (*(ITEM **) b)->distance,
131+
return strncmp(operand + (*(ITEM **) a)->distance,
132+
operand + (*(ITEM **) b)->distance,
133133
(*(ITEM **) b)->length);
134134

135135
return ((*(ITEM **) a)->length > (*(ITEM **) b)->length) ? 1 : -1;
@@ -158,15 +158,14 @@ SortAndUniqItems(char *operand, ITEM * item, int *size)
158158
if (*size < 2)
159159
return res;
160160

161-
SortAndUniqOperand = operand;
162-
qsort(res, *size, sizeof(ITEM **), compareITEM);
161+
qsort_arg(res, *size, sizeof(ITEM **), compareITEM, (void *) operand);
163162

164163
ptr = res + 1;
165164
prevptr = res;
166165

167166
while (ptr - res < *size)
168167
{
169-
if (compareITEM((void *) ptr, (void *) prevptr) != 0)
168+
if (compareITEM((void *) ptr, (void *) prevptr, (void *) operand) != 0)
170169
{
171170
prevptr++;
172171
*prevptr = *ptr;
@@ -551,10 +550,11 @@ get_docrep(tsvector * txt, QUERYTYPE * query, int *doclen)
551550
int len = query->size * 4,
552551
cur = 0;
553552
DocRepresentation *doc;
553+
char *operand;
554554

555555
*(uint16 *) POSNULL = lengthof(POSNULL) - 1;
556556
doc = (DocRepresentation *) palloc(sizeof(DocRepresentation) * len);
557-
SortAndUniqOperand = GETOPERAND(query);
557+
operand = GETOPERAND(query);
558558
reset_istrue_flag(query);
559559

560560
for (i = 0; i < query->size; i++)
@@ -598,7 +598,9 @@ get_docrep(tsvector * txt, QUERYTYPE * query, int *doclen)
598598
for (k = 0; k < query->size; k++)
599599
{
600600
kptr = item + k;
601-
if (k == i || (item[k].type == VAL && compareITEM(&kptr, &iptr) == 0))
601+
if (k == i ||
602+
(item[k].type == VAL &&
603+
compareITEM(&kptr, &iptr, operand) == 0))
602604
{
603605
doc[cur].item[doc[cur].nitem] = item + k;
604606
doc[cur].nitem++;

‎contrib/tsearch2/tsvector.c

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -85,14 +85,14 @@ uniquePos(WordEntryPos * a, int4 l)
8585
return res + 1 - a;
8686
}
8787

88-
static char *BufferStr;
8988
static int
90-
compareentry(const void *a, const void *b)
89+
compareentry(const void *a, const void *b, void *arg)
9190
{
91+
char *BufferStr = (char *) arg;
92+
9293
if (((WordEntryIN *) a)->entry.len == ((WordEntryIN *) b)->entry.len)
9394
{
94-
return strncmp(
95-
&BufferStr[((WordEntryIN *) a)->entry.pos],
95+
return strncmp(&BufferStr[((WordEntryIN *) a)->entry.pos],
9696
&BufferStr[((WordEntryIN *) b)->entry.pos],
9797
((WordEntryIN *) a)->entry.len);
9898
}
@@ -117,8 +117,7 @@ uniqueentry(WordEntryIN * a, int4 l, char *buf, int4 *outbuflen)
117117
}
118118

119119
ptr = a + 1;
120-
BufferStr = buf;
121-
qsort((void *) a, l, sizeof(WordEntryIN), compareentry);
120+
qsort_arg((void *) a, l, sizeof(WordEntryIN), compareentry, (void *) buf);
122121

123122
while (ptr - a < l)
124123
{

‎src/backend/access/gin/ginutil.c

Lines changed: 25 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
* Portions Copyright (c) 1994, Regents of the University of California
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/access/gin/ginutil.c,v 1.7 2006/10/04 00:29:48 momjian Exp $
11+
* $PostgreSQL: pgsql/src/backend/access/gin/ginutil.c,v 1.8 2006/10/05 17:57:40 tgl Exp $
1212
*-------------------------------------------------------------------------
1313
*/
1414

@@ -129,62 +129,48 @@ compareEntries(GinState *ginstate, Datum a, Datum b)
129129
);
130130
}
131131

132-
static FmgrInfo *cmpDatumPtr = NULL;
133-
134-
#if defined(__INTEL_COMPILER) && (defined(__ia64__) || defined(__ia64))
135-
/*
136-
* Intel Compiler on Intel Itanium with -O2 has a bug around
137-
* change static variable by user function called from
138-
* libc func: it doesn't change. So mark it as volatile.
139-
*
140-
* It's a pity, but it's impossible to define optimization
141-
* level here.
142-
*/
143-
#define VOLATILE volatile
144-
#else
145-
#define VOLATILE
146-
#endif
147-
148-
static bool VOLATILE needUnique = FALSE;
132+
typedef struct
133+
{
134+
FmgrInfo *cmpDatumFunc;
135+
bool *needUnique;
136+
} cmpEntriesData;
149137

150138
static int
151-
cmpEntries(const void *a, const void *b)
139+
cmpEntries(const Datum *a, const Datum *b, cmpEntriesData *arg)
152140
{
153-
int res = DatumGetInt32(
154-
FunctionCall2(
155-
cmpDatumPtr,
156-
*(Datum *) a,
157-
*(Datum *) b
158-
)
159-
);
141+
int res = DatumGetInt32(FunctionCall2(arg->cmpDatumFunc,
142+
*a, *b));
160143

161144
if (res == 0)
162-
needUnique = TRUE;
145+
*(arg->needUnique) = TRUE;
163146

164147
return res;
165148
}
166149

167150
Datum *
168-
extractEntriesS(GinState *ginstate, Datum value, uint32 *nentries)
151+
extractEntriesS(GinState *ginstate, Datum value, uint32 *nentries,
152+
bool *needUnique)
169153
{
170154
Datum *entries;
171155

172-
entries = (Datum *) DatumGetPointer(
173-
FunctionCall2(
156+
entries = (Datum *) DatumGetPointer(FunctionCall2(
174157
&ginstate->extractValueFn,
175158
value,
176159
PointerGetDatum(nentries)
177-
)
178-
);
160+
));
179161

180162
if (entries == NULL)
181163
*nentries = 0;
182164

165+
*needUnique = FALSE;
183166
if (*nentries > 1)
184167
{
185-
cmpDatumPtr = &ginstate->compareFn;
186-
needUnique = FALSE;
187-
qsort(entries, *nentries, sizeof(Datum), cmpEntries);
168+
cmpEntriesData arg;
169+
170+
arg.cmpDatumFunc = &ginstate->compareFn;
171+
arg.needUnique = needUnique;
172+
qsort_arg(entries, *nentries, sizeof(Datum),
173+
(qsort_arg_comparator) cmpEntries, (void *) &arg);
188174
}
189175

190176
return entries;
@@ -194,9 +180,11 @@ extractEntriesS(GinState *ginstate, Datum value, uint32 *nentries)
194180
Datum *
195181
extractEntriesSU(GinState *ginstate, Datum value, uint32 *nentries)
196182
{
197-
Datum *entries = extractEntriesS(ginstate, value, nentries);
183+
bool needUnique;
184+
Datum *entries = extractEntriesS(ginstate, value, nentries,
185+
&needUnique);
198186

199-
if (*nentries > 1 && needUnique)
187+
if (needUnique)
200188
{
201189
Datum *ptr,
202190
*res;

‎src/backend/commands/analyze.c

Lines changed: 24 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/commands/analyze.c,v 1.99 2006/10/04 00:29:50 momjian Exp $
11+
* $PostgreSQL: pgsql/src/backend/commands/analyze.c,v 1.100 2006/10/05 17:57:40 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -1294,11 +1294,12 @@ typedef struct
12941294
int first; /* values[] index of first occurrence */
12951295
} ScalarMCVItem;
12961296

1297-
1298-
/* context information for compare_scalars() */
1299-
static FmgrInfo *datumCmpFn;
1300-
static SortFunctionKind datumCmpFnKind;
1301-
static int *datumCmpTupnoLink;
1297+
typedef struct
1298+
{
1299+
FmgrInfo *cmpFn;
1300+
SortFunctionKind cmpFnKind;
1301+
int *tupnoLink;
1302+
} CompareScalarsContext;
13021303

13031304

13041305
static void compute_minimal_stats(VacAttrStatsP stats,
@@ -1309,7 +1310,7 @@ static void compute_scalar_stats(VacAttrStatsP stats,
13091310
AnalyzeAttrFetchFunc fetchfunc,
13101311
int samplerows,
13111312
double totalrows);
1312-
static int compare_scalars(const void *a, const void *b);
1313+
static int compare_scalars(const void *a, const void *b, void *arg);
13131314
static int compare_mcvs(const void *a, const void *b);
13141315

13151316

@@ -1828,13 +1829,14 @@ compute_scalar_stats(VacAttrStatsP stats,
18281829
num_hist,
18291830
dups_cnt;
18301831
int slot_idx = 0;
1832+
CompareScalarsContext cxt;
18311833

18321834
/* Sort the collected values */
1833-
datumCmpFn = &f_cmpfn;
1834-
datumCmpFnKind = cmpFnKind;
1835-
datumCmpTupnoLink = tupnoLink;
1836-
qsort((void *) values, values_cnt,
1837-
sizeof(ScalarItem), compare_scalars);
1835+
cxt.cmpFn = &f_cmpfn;
1836+
cxt.cmpFnKind = cmpFnKind;
1837+
cxt.tupnoLink = tupnoLink;
1838+
qsort_arg((void *) values, values_cnt, sizeof(ScalarItem),
1839+
compare_scalars, (void *) &cxt);
18381840

18391841
/*
18401842
* Now scan the values in order, find the most common ones, and also
@@ -2183,35 +2185,36 @@ compute_scalar_stats(VacAttrStatsP stats,
21832185
}
21842186

21852187
/*
2186-
* qsort comparator for sorting ScalarItems
2188+
* qsort_arg comparator for sorting ScalarItems
21872189
*
2188-
* Aside from sorting the items, we update the datumCmpTupnoLink[] array
2190+
* Aside from sorting the items, we update the tupnoLink[] array
21892191
* whenever two ScalarItems are found to contain equal datums. The array
21902192
* is indexed by tupno; for each ScalarItem, it contains the highest
21912193
* tupno that that item's datum has been found to be equal to. This allows
21922194
* us to avoid additional comparisons in compute_scalar_stats().
21932195
*/
21942196
static int
2195-
compare_scalars(const void *a, const void *b)
2197+
compare_scalars(const void *a, const void *b, void *arg)
21962198
{
21972199
Datum da = ((ScalarItem *) a)->value;
21982200
int ta = ((ScalarItem *) a)->tupno;
21992201
Datum db = ((ScalarItem *) b)->value;
22002202
int tb = ((ScalarItem *) b)->tupno;
2203+
CompareScalarsContext *cxt = (CompareScalarsContext *) arg;
22012204
int32 compare;
22022205

2203-
compare = ApplySortFunction(datumCmpFn, datumCmpFnKind,
2206+
compare = ApplySortFunction(cxt->cmpFn, cxt->cmpFnKind,
22042207
da, false, db, false);
22052208
if (compare != 0)
22062209
return compare;
22072210

22082211
/*
2209-
* The two datums are equal, so update datumCmpTupnoLink[].
2212+
* The two datums are equal, so update cxt->tupnoLink[].
22102213
*/
2211-
if (datumCmpTupnoLink[ta] < tb)
2212-
datumCmpTupnoLink[ta] = tb;
2213-
if (datumCmpTupnoLink[tb] < ta)
2214-
datumCmpTupnoLink[tb] = ta;
2214+
if (cxt->tupnoLink[ta] < tb)
2215+
cxt->tupnoLink[ta] = tb;
2216+
if (cxt->tupnoLink[tb] < ta)
2217+
cxt->tupnoLink[tb] = ta;
22152218

22162219
/*
22172220
* For equal datums, sort by tupno

‎src/include/access/gin.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
* header file for postgres inverted index access method implementation.
44
*
55
* Copyright (c) 2006, PostgreSQL Global Development Group
6-
* $PostgreSQL: pgsql/src/include/access/gin.h,v 1.8 2006/10/04 00:30:06 momjian Exp $
6+
* $PostgreSQL: pgsql/src/include/access/gin.h,v 1.9 2006/10/05 17:57:40 tgl Exp $
77
*--------------------------------------------------------------------------
88
*/
99

@@ -232,7 +232,8 @@ extern Buffer GinNewBuffer(Relation index);
232232
extern void GinInitBuffer(Buffer b, uint32 f);
233233
extern void GinInitPage(Page page, uint32 f, Size pageSize);
234234
extern int compareEntries(GinState *ginstate, Datum a, Datum b);
235-
extern Datum *extractEntriesS(GinState *ginstate, Datum value, uint32 *nentries);
235+
extern Datum *extractEntriesS(GinState *ginstate, Datum value,
236+
uint32 *nentries, bool *needUnique);
236237
extern Datum *extractEntriesSU(GinState *ginstate, Datum value, uint32 *nentries);
237238
extern Page GinPageGetCopyPage(Page page);
238239

0 commit comments

Comments
 (0)