Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit5d7a555

Browse files
committed
Code review for recent libpq changes. Be more careful about error
handling in SIGPIPE processing; avoid unnecessary pollution of applicationlink-symbol namespace; spell 'pointer to function' in the conventionalway.
1 parent9b711e7 commit5d7a555

File tree

5 files changed

+90
-71
lines changed

5 files changed

+90
-71
lines changed

‎src/interfaces/libpq/fe-connect.c

Lines changed: 12 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/interfaces/libpq/fe-connect.c,v 1.291 2004/12/0215:32:54 momjian Exp $
11+
* $PostgreSQL: pgsql/src/interfaces/libpq/fe-connect.c,v 1.292 2004/12/0223:20:19 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -202,6 +202,11 @@ static int parseServiceInfo(PQconninfoOption *options,
202202
staticchar*pwdfMatchesString(char*buf,char*token);
203203
staticchar*PasswordFromFile(char*hostname,char*port,char*dbname,
204204
char*username);
205+
staticvoiddefault_threadlock(intacquire);
206+
207+
208+
/* global variable because fe-auth.c needs to access it */
209+
pgthreadlock_tpg_g_threadlock=default_threadlock;
205210

206211

207212
/*
@@ -3276,16 +3281,6 @@ PasswordFromFile(char *hostname, char *port, char *dbname, char *username)
32763281
* if they are not required.
32773282
*/
32783283

3279-
void
3280-
PQinitSSL(intdo_init)
3281-
{
3282-
#ifdefUSE_SSL
3283-
pq_initssllib=do_init;
3284-
#endif
3285-
}
3286-
3287-
staticpgthreadlock_tdefault_threadlock;
3288-
32893284
staticvoid
32903285
default_threadlock(intacquire)
32913286
{
@@ -3313,17 +3308,15 @@ default_threadlock(int acquire)
33133308
#endif
33143309
}
33153310

3316-
pgthreadlock_t*g_threadlock=default_threadlock;
3317-
3318-
pgthreadlock_t*
3319-
PQregisterThreadLock(pgthreadlock_t*newhandler)
3311+
pgthreadlock_t
3312+
PQregisterThreadLock(pgthreadlock_tnewhandler)
33203313
{
3321-
pgthreadlock_t*prev;
3314+
pgthreadlock_tprev=pg_g_threadlock;
33223315

3323-
prev=g_threadlock;
33243316
if (newhandler)
3325-
g_threadlock=newhandler;
3317+
pg_g_threadlock=newhandler;
33263318
else
3327-
g_threadlock=default_threadlock;
3319+
pg_g_threadlock=default_threadlock;
3320+
33283321
returnprev;
33293322
}

‎src/interfaces/libpq/fe-print.c

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
* didn't really belong there.
1111
*
1212
* IDENTIFICATION
13-
* $PostgreSQL: pgsql/src/interfaces/libpq/fe-print.c,v 1.56 2004/12/0215:32:54 momjian Exp $
13+
* $PostgreSQL: pgsql/src/interfaces/libpq/fe-print.c,v 1.57 2004/12/0223:20:20 tgl Exp $
1414
*
1515
*-------------------------------------------------------------------------
1616
*/
@@ -193,8 +193,8 @@ PQprint(FILE *fout,
193193
{
194194
usePipe=1;
195195
#ifdefENABLE_THREAD_SAFETY
196-
pq_block_sigpipe(&osigset,&sigpipe_pending);
197-
sigpipe_masked= true;
196+
if (pq_block_sigpipe(&osigset,&sigpipe_pending)==0)
197+
sigpipe_masked= true;
198198
#else
199199
#ifndefWIN32
200200
oldsigpipehandler=pqsignal(SIGPIPE,SIG_IGN);
@@ -316,8 +316,9 @@ PQprint(FILE *fout,
316316
pclose(fout);
317317
#endif
318318
#ifdefENABLE_THREAD_SAFETY
319+
/* we can't easily verify if EPIPE occurred, so say it did */
319320
if (sigpipe_masked)
320-
pq_reset_sigpipe(&osigset,sigpipe_pending);
321+
pq_reset_sigpipe(&osigset,sigpipe_pending, true);
321322
#else
322323
#ifndefWIN32
323324
pqsignal(SIGPIPE,oldsigpipehandler);

‎src/interfaces/libpq/fe-secure.c

Lines changed: 58 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
*
1212
*
1313
* IDENTIFICATION
14-
* $PostgreSQL: pgsql/src/interfaces/libpq/fe-secure.c,v 1.58 2004/12/0215:32:54 momjian Exp $
14+
* $PostgreSQL: pgsql/src/interfaces/libpq/fe-secure.c,v 1.59 2004/12/0223:20:21 tgl Exp $
1515
*
1616
* NOTES
1717
* [ Most of these notes are wrong/obsolete, but perhaps not all ]
@@ -147,7 +147,7 @@ static void SSLerrfree(char *buf);
147147
#endif
148148

149149
#ifdefUSE_SSL
150-
boolpq_initssllib= true;
150+
staticboolpq_initssllib= true;
151151

152152
staticSSL_CTX*SSL_context=NULL;
153153
#endif
@@ -212,6 +212,19 @@ KWbuHn491xNO25CQWMtem80uKw+pTnisBRF/454n1Jnhub144YRBoN8CAQI=\n\
212212
/* Procedures common to all secure sessions*/
213213
/* ------------------------------------------------------------ */
214214

215+
216+
/*
217+
* Exported (but as yet undocumented) function to allow application to
218+
* tell us it's already initialized OpenSSL.
219+
*/
220+
void
221+
PQinitSSL(intdo_init)
222+
{
223+
#ifdefUSE_SSL
224+
pq_initssllib=do_init;
225+
#endif
226+
}
227+
215228
/*
216229
*Initialize global context
217230
*/
@@ -377,8 +390,10 @@ pqsecure_write(PGconn *conn, const void *ptr, size_t len)
377390
#ifdefENABLE_THREAD_SAFETY
378391
sigset_tosigmask;
379392
boolsigpipe_pending;
393+
boolgot_epipe= false;
380394

381-
pq_block_sigpipe(&osigmask,&sigpipe_pending);
395+
if (pq_block_sigpipe(&osigmask,&sigpipe_pending)<0)
396+
return-1;
382397
#else
383398
#ifndefWIN32
384399
pqsigfuncoldsighandler=pqsignal(SIGPIPE,SIG_IGN);
@@ -413,9 +428,13 @@ pqsecure_write(PGconn *conn, const void *ptr, size_t len)
413428
charsebuf[256];
414429

415430
if (n==-1)
431+
{
432+
if (SOCK_ERRNO==EPIPE)
433+
got_epipe= true;
416434
printfPQExpBuffer(&conn->errorMessage,
417435
libpq_gettext("SSL SYSCALL error: %s\n"),
418436
SOCK_STRERROR(SOCK_ERRNO,sebuf,sizeof(sebuf)));
437+
}
419438
else
420439
{
421440
printfPQExpBuffer(&conn->errorMessage,
@@ -448,15 +467,16 @@ pqsecure_write(PGconn *conn, const void *ptr, size_t len)
448467
}
449468
else
450469
#endif
470+
{
451471
n=send(conn->sock,ptr,len,0);
452-
/*
453-
*Possible optimization:ifsigpending() turns out to be an
454-
*expensive operation, we can set sigpipe_pending = 'true'
455-
*here if errno != EPIPE, avoiding a sigpending call.
456-
*/
472+
#ifdefENABLE_THREAD_SAFETY
473+
if(n<0&&SOCK_ERRNO==EPIPE)
474+
got_epipe=true;
475+
#endif
476+
}
457477

458478
#ifdefENABLE_THREAD_SAFETY
459-
pq_reset_sigpipe(&osigmask,sigpipe_pending);
479+
pq_reset_sigpipe(&osigmask,sigpipe_pending,got_epipe);
460480
#else
461481
#ifndefWIN32
462482
pqsignal(SIGPIPE,oldsighandler);
@@ -1228,13 +1248,14 @@ pq_block_sigpipe(sigset_t *osigset, bool *sigpipe_pending)
12281248
{
12291249
sigset_tsigpipe_sigset;
12301250
sigset_tsigset;
1231-
intret;
12321251

12331252
sigemptyset(&sigpipe_sigset);
12341253
sigaddset(&sigpipe_sigset,SIGPIPE);
12351254

12361255
/* Block SIGPIPE and save previous mask for later reset */
1237-
ret=pthread_sigmask(SIG_BLOCK,&sigpipe_sigset,osigset);
1256+
SOCK_ERRNO_SET(pthread_sigmask(SIG_BLOCK,&sigpipe_sigset,osigset));
1257+
if (SOCK_ERRNO)
1258+
return-1;
12381259

12391260
/* We can have a pending SIGPIPE only if it was blocked before */
12401261
if (sigismember(osigset,SIGPIPE))
@@ -1251,44 +1272,52 @@ pq_block_sigpipe(sigset_t *osigset, bool *sigpipe_pending)
12511272
else
12521273
*sigpipe_pending= false;
12531274

1254-
returnret;
1275+
return0;
12551276
}
12561277

12571278
/*
12581279
*Discard any pending SIGPIPE and reset the signal mask.
1259-
*We might be discarding a blocked SIGPIPE that we didn't generate,
1260-
*but we document that you can't keep blocked SIGPIPE calls across
1261-
*libpq function calls.
1280+
*
1281+
* Note: we are effectively assuming here that the C library doesn't queue
1282+
* up multiple SIGPIPE events. If it did, then we'd accidentally leave
1283+
* ours in the queue when an event was already pending and we got another.
1284+
* As long as it doesn't queue multiple events, we're OK because the caller
1285+
* can't tell the difference.
1286+
*
1287+
* The caller should say got_epipe = FALSE if it is certain that it
1288+
* didn't get an EPIPE error; in that case we'll skip the clear operation
1289+
* and things are definitely OK, queuing or no. If it got one or might have
1290+
* gotten one, pass got_epipe = TRUE.
1291+
*
1292+
* We do not want this to change errno, since if it did that could lose
1293+
* the error code from a preceding send(). We essentially assume that if
1294+
* we were able to do pq_block_sigpipe(), this can't fail.
12621295
*/
1263-
int
1264-
pq_reset_sigpipe(sigset_t*osigset,boolsigpipe_pending)
1296+
void
1297+
pq_reset_sigpipe(sigset_t*osigset,boolsigpipe_pending,boolgot_epipe)
12651298
{
1299+
intsave_errno=SOCK_ERRNO;
12661300
intsigno;
12671301
sigset_tsigset;
12681302

12691303
/* Clear SIGPIPE only if none was pending */
1270-
if (!sigpipe_pending)
1304+
if (got_epipe&&!sigpipe_pending)
12711305
{
1272-
if (sigpending(&sigset)!=0)
1273-
return-1;
1274-
1275-
/*
1276-
*Discard pending and blocked SIGPIPE
1277-
*/
1278-
if (sigismember(&sigset,SIGPIPE))
1306+
if (sigpending(&sigset)==0&&
1307+
sigismember(&sigset,SIGPIPE))
12791308
{
12801309
sigset_tsigpipe_sigset;
12811310

12821311
sigemptyset(&sigpipe_sigset);
12831312
sigaddset(&sigpipe_sigset,SIGPIPE);
1284-
1313+
12851314
sigwait(&sigpipe_sigset,&signo);
1286-
if (signo!=SIGPIPE)
1287-
return-1;
12881315
}
12891316
}
12901317

12911318
/* Restore saved block mask */
1292-
returnpthread_sigmask(SIG_SETMASK,osigset,NULL);
1319+
pthread_sigmask(SIG_SETMASK,osigset,NULL);
1320+
1321+
SOCK_ERRNO_SET(save_errno);
12931322
}
12941323
#endif

‎src/interfaces/libpq/libpq-fe.h

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* Portions Copyright (c) 1996-2004, PostgreSQL Global Development Group
88
* Portions Copyright (c) 1994, Regents of the University of California
99
*
10-
* $PostgreSQL: pgsql/src/interfaces/libpq/libpq-fe.h,v 1.114 2004/12/0215:32:54 momjian Exp $
10+
* $PostgreSQL: pgsql/src/interfaces/libpq/libpq-fe.h,v 1.115 2004/12/0223:20:21 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -279,6 +279,9 @@ extern SSL *PQgetssl(PGconn *conn);
279279
externvoid*PQgetssl(PGconn*conn);
280280
#endif
281281

282+
/* Tell libpq whether it needs to initialize OpenSSL */
283+
externvoidPQinitSSL(intdo_init);
284+
282285
/* Set verbosity for PQerrorMessage and PQresultErrorMessage */
283286
externPGVerbosityPQsetErrorVerbosity(PGconn*conn,PGVerbosityverbosity);
284287

@@ -301,11 +304,9 @@ extern PQnoticeProcessor PQsetNoticeProcessor(PGconn *conn,
301304
* Only required for multithreaded apps that use kerberos
302305
* both within their app and for postgresql connections.
303306
*/
304-
typedefvoid (pgthreadlock_t) (intacquire);
307+
typedefvoid (*pgthreadlock_t) (intacquire);
305308

306-
externpgthreadlock_t*PQregisterThreadLock(pgthreadlock_t*newhandler);
307-
308-
externvoidPQinitSSL(intdo_init);
309+
externpgthreadlock_tPQregisterThreadLock(pgthreadlock_tnewhandler);
309310

310311
/* === in fe-exec.c === */
311312

@@ -495,8 +496,6 @@ extern intPQdsplen(const unsigned char *s, int encoding);
495496
/* Get encoding id from environment variable PGCLIENTENCODING */
496497
externintPQenv2encoding(void);
497498

498-
/* === in fe-secure.c === */
499-
500499
#ifdef__cplusplus
501500
}
502501
#endif

‎src/interfaces/libpq/libpq-int.h

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
* Portions Copyright (c) 1996-2004, PostgreSQL Global Development Group
1313
* Portions Copyright (c) 1994, Regents of the University of California
1414
*
15-
* $PostgreSQL: pgsql/src/interfaces/libpq/libpq-int.h,v 1.97 2004/12/0215:32:54 momjian Exp $
15+
* $PostgreSQL: pgsql/src/interfaces/libpq/libpq-int.h,v 1.98 2004/12/0223:20:21 tgl Exp $
1616
*
1717
*-------------------------------------------------------------------------
1818
*/
@@ -379,13 +379,13 @@ extern int pqPacketSend(PGconn *conn, char pack_type,
379379
constvoid*buf,size_tbuf_len);
380380

381381
#ifdefENABLE_THREAD_SAFETY
382-
externpgthreadlock_t*g_threadlock;
382+
externpgthreadlock_tpg_g_threadlock;
383383

384-
#definepglock_thread() g_threadlock(true);
385-
#definepgunlock_thread() g_threadlock(false);
384+
#definepglock_thread()pg_g_threadlock(true)
385+
#definepgunlock_thread()pg_g_threadlock(false)
386386
#else
387-
#definepglock_thread()((void)0)
388-
#definepgunlock_thread()((void)0)
387+
#definepglock_thread()((void)0)
388+
#definepgunlock_thread()((void)0)
389389
#endif
390390

391391

@@ -476,13 +476,10 @@ extern void pqsecure_close(PGconn *);
476476
externssize_tpqsecure_read(PGconn*,void*ptr,size_tlen);
477477
externssize_tpqsecure_write(PGconn*,constvoid*ptr,size_tlen);
478478

479-
#ifdefUSE_SSL
480-
externboolpq_initssllib;
481-
#endif
482-
483479
#ifdefENABLE_THREAD_SAFETY
484-
intpq_block_sigpipe(sigset_t*osigset,bool*sigpipe_pending);
485-
intpq_reset_sigpipe(sigset_t*osigset,boolsigpipe_pending);
480+
externintpq_block_sigpipe(sigset_t*osigset,bool*sigpipe_pending);
481+
externvoidpq_reset_sigpipe(sigset_t*osigset,boolsigpipe_pending,
482+
boolgot_epipe);
486483
#endif
487484

488485
/*

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp