Skip to content

Commit d6c55de

Browse files
tglsfdctglsfdc
committed
Implement %m in src/port/snprintf.c, and teach elog.c to rely on that.
I started out with the idea that we needed to detect use of %m format specs in contexts other than elog/ereport calls, because we couldn't rely on that working in *printf calls. But a better answer is to fix things so that it does work. Now that we're using snprintf.c all the time, we can implement %m in that and we've fixed the problem. This requires also adjusting our various printf-wrapping functions so that they ensure "errno" is preserved when they call snprintf.c. Remove elog.c's handmade implementation of %m, and let it rely on snprintf to support the feature. That should provide some performance gain, though I've not attempted to measure it. There are a lot of places where we could now simplify 'printf("%s", strerror(errno))' into 'printf("%m")', but I'm not in any big hurry to make that happen. Patch by me, reviewed by Michael Paquier Discussion: https://postgr.es/m/2975.1526862605@sss.pgh.pa.us
1 parent 96bf88d commit d6c55de

File tree

8 files changed

+57
-67
lines changed

8 files changed

+57
-67
lines changed

‎src/backend/lib/stringinfo.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,12 +77,15 @@ resetStringInfo(StringInfo str)
7777
void
7878
appendStringInfo(StringInfo str, const char *fmt,...)
7979
{
80+
int save_errno = errno;
81+
8082
for (;;)
8183
{
8284
va_list args;
8385
int needed;
8486

8587
/* Try to format the data. */
88+
errno = save_errno;
8689
va_start(args, fmt);
8790
needed = appendStringInfoVA(str, fmt, args);
8891
va_end(args);
@@ -105,6 +108,9 @@ appendStringInfo(StringInfo str, const char *fmt,...)
105108
* pass the return value to enlargeStringInfo() before trying again; see
106109
* appendStringInfo for standard usage pattern.
107110
*
111+
* Caution: callers must be sure to preserve their entry-time errno
112+
* when looping, in case the fmt contains "%m".
113+
*
108114
* XXX This API is ugly, but there seems no alternative given the C spec's
109115
* restrictions on what can portably be done with va_list arguments: you have
110116
* to redo va_start before you can rescan the argument list, and we can't do

‎src/backend/utils/error/elog.c

Lines changed: 4 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,6 @@ static void write_csvlog(ErrorData *edata);
177177
static void send_message_to_server_log(ErrorData *edata);
178178
static void write_pipe_chunks(char *data, int len, int dest);
179179
static void send_message_to_frontend(ErrorData *edata);
180-
static char *expand_fmt_string(const char *fmt, ErrorData *edata);
181180
static const char *error_severity(int elevel);
182181
static void append_with_tabs(StringInfo buf, const char *str);
183182
static bool is_log_level_output(int elevel, int log_min_level);
@@ -705,13 +704,10 @@ errcode_for_socket_access(void)
705704
*/
706705
#define EVALUATE_MESSAGE(domain, targetfield, appendval, translateit) \
707706
{ \
708-
char *fmtbuf; \
709707
StringInfoData buf; \
710708
/* Internationalize the error format string */ \
711709
if ((translateit) && !in_error_recursion_trouble()) \
712710
fmt = dgettext((domain), fmt); \
713-
/* Expand %m in format string */ \
714-
fmtbuf = expand_fmt_string(fmt, edata); \
715711
initStringInfo(&buf); \
716712
if ((appendval) && edata->targetfield) { \
717713
appendStringInfoString(&buf, edata->targetfield); \
@@ -722,15 +718,14 @@ errcode_for_socket_access(void)
722718
{ \
723719
va_list args; \
724720
int needed; \
721+
errno = edata->saved_errno; \
725722
va_start(args, fmt); \
726-
needed = appendStringInfoVA(&buf, fmtbuf, args); \
723+
needed = appendStringInfoVA(&buf, fmt, args); \
727724
va_end(args); \
728725
if (needed == 0) \
729726
break; \
730727
enlargeStringInfo(&buf, needed); \
731728
} \
732-
/* Done with expanded fmt */ \
733-
pfree(fmtbuf); \
734729
/* Save the completed message into the stack item */ \
735730
if (edata->targetfield) \
736731
pfree(edata->targetfield); \
@@ -746,15 +741,12 @@ errcode_for_socket_access(void)
746741
#define EVALUATE_MESSAGE_PLURAL(domain, targetfield, appendval) \
747742
{ \
748743
const char *fmt; \
749-
char *fmtbuf; \
750744
StringInfoData buf; \
751745
/* Internationalize the error format string */ \
752746
if (!in_error_recursion_trouble()) \
753747
fmt = dngettext((domain), fmt_singular, fmt_plural, n); \
754748
else \
755749
fmt = (n == 1 ? fmt_singular : fmt_plural); \
756-
/* Expand %m in format string */ \
757-
fmtbuf = expand_fmt_string(fmt, edata); \
758750
initStringInfo(&buf); \
759751
if ((appendval) && edata->targetfield) { \
760752
appendStringInfoString(&buf, edata->targetfield); \
@@ -765,15 +757,14 @@ errcode_for_socket_access(void)
765757
{ \
766758
va_list args; \
767759
int needed; \
760+
errno = edata->saved_errno; \
768761
va_start(args, n); \
769-
needed = appendStringInfoVA(&buf, fmtbuf, args); \
762+
needed = appendStringInfoVA(&buf, fmt, args); \
770763
va_end(args); \
771764
if (needed == 0) \
772765
break; \
773766
enlargeStringInfo(&buf, needed); \
774767
} \
775-
/* Done with expanded fmt */ \
776-
pfree(fmtbuf); \
777768
/* Save the completed message into the stack item */ \
778769
if (edata->targetfield) \
779770
pfree(edata->targetfield); \
@@ -3328,59 +3319,6 @@ send_message_to_frontend(ErrorData *edata)
33283319
*/
33293320

33303321

3331-
/*
3332-
* expand_fmt_string --- process special format codes in a format string
3333-
*
3334-
* We must replace %m with the appropriate strerror string, since vsnprintf
3335-
* won't know what to do with it.
3336-
*
3337-
* The result is a palloc'd string.
3338-
*/
3339-
static char *
3340-
expand_fmt_string(const char *fmt, ErrorData *edata)
3341-
{
3342-
StringInfoData buf;
3343-
const char *cp;
3344-
3345-
initStringInfo(&buf);
3346-
3347-
for (cp = fmt; *cp; cp++)
3348-
{
3349-
if (cp[0] == '%' && cp[1] != '\0')
3350-
{
3351-
cp++;
3352-
if (*cp == 'm')
3353-
{
3354-
/*
3355-
* Replace %m by system error string. If there are any %'s in
3356-
* the string, we'd better double them so that vsnprintf won't
3357-
* misinterpret.
3358-
*/
3359-
const char *cp2;
3360-
3361-
cp2 = strerror(edata->saved_errno);
3362-
for (; *cp2; cp2++)
3363-
{
3364-
if (*cp2 == '%')
3365-
appendStringInfoCharMacro(&buf, '%');
3366-
appendStringInfoCharMacro(&buf, *cp2);
3367-
}
3368-
}
3369-
else
3370-
{
3371-
/* copy % and next char --- this avoids trouble with %%m */
3372-
appendStringInfoCharMacro(&buf, '%');
3373-
appendStringInfoCharMacro(&buf, *cp);
3374-
}
3375-
}
3376-
else
3377-
appendStringInfoCharMacro(&buf, *cp);
3378-
}
3379-
3380-
return buf.data;
3381-
}
3382-
3383-
33843322
/*
33853323
* error_severity --- get string representing elevel
33863324
*

‎src/bin/pg_dump/pg_backup_archiver.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1507,6 +1507,7 @@ archputs(const char *s, Archive *AH)
15071507
int
15081508
archprintf(Archive *AH, const char *fmt,...)
15091509
{
1510+
int save_errno = errno;
15101511
char *p;
15111512
size_t len = 128; /* initial assumption about buffer size */
15121513
size_t cnt;
@@ -1519,6 +1520,7 @@ archprintf(Archive *AH, const char *fmt,...)
15191520
p = (char *) pg_malloc(len);
15201521

15211522
/* Try to format the data. */
1523+
errno = save_errno;
15221524
va_start(args, fmt);
15231525
cnt = pvsnprintf(p, len, fmt, args);
15241526
va_end(args);
@@ -1640,6 +1642,7 @@ RestoreOutput(ArchiveHandle *AH, OutputContext savedContext)
16401642
int
16411643
ahprintf(ArchiveHandle *AH, const char *fmt,...)
16421644
{
1645+
int save_errno = errno;
16431646
char *p;
16441647
size_t len = 128; /* initial assumption about buffer size */
16451648
size_t cnt;
@@ -1652,6 +1655,7 @@ ahprintf(ArchiveHandle *AH, const char *fmt,...)
16521655
p = (char *) pg_malloc(len);
16531656

16541657
/* Try to format the data. */
1658+
errno = save_errno;
16551659
va_start(args, fmt);
16561660
cnt = pvsnprintf(p, len, fmt, args);
16571661
va_end(args);

‎src/bin/pg_dump/pg_backup_tar.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1026,6 +1026,7 @@ _EndBlobs(ArchiveHandle *AH, TocEntry *te)
10261026
static int
10271027
tarPrintf(ArchiveHandle *AH, TAR_MEMBER *th, const char *fmt,...)
10281028
{
1029+
int save_errno = errno;
10291030
char *p;
10301031
size_t len = 128; /* initial assumption about buffer size */
10311032
size_t cnt;
@@ -1038,6 +1039,7 @@ tarPrintf(ArchiveHandle *AH, TAR_MEMBER *th, const char *fmt,...)
10381039
p = (char *) pg_malloc(len);
10391040

10401041
/* Try to format the data. */
1042+
errno = save_errno;
10411043
va_start(args, fmt);
10421044
cnt = pvsnprintf(p, len, fmt, args);
10431045
va_end(args);

‎src/common/psprintf.c

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
char *
4646
psprintf(const char *fmt,...)
4747
{
48+
int save_errno = errno;
4849
size_t len = 128; /* initial assumption about buffer size */
4950

5051
for (;;)
@@ -60,6 +61,7 @@ psprintf(const char *fmt,...)
6061
result = (char *) palloc(len);
6162

6263
/* Try to format the data. */
64+
errno = save_errno;
6365
va_start(args, fmt);
6466
newlen = pvsnprintf(result, len, fmt, args);
6567
va_end(args);
@@ -89,6 +91,9 @@ psprintf(const char *fmt,...)
8991
* Other error cases do not return, but exit via elog(ERROR) or exit().
9092
* Hence, this shouldn't be used inside libpq.
9193
*
94+
* Caution: callers must be sure to preserve their entry-time errno
95+
* when looping, in case the fmt contains "%m".
96+
*
9297
* Note that the semantics of the return value are not exactly C99's.
9398
* First, we don't promise that the estimated buffer size is exactly right;
9499
* callers must be prepared to loop multiple times to get the right size.

‎src/interfaces/libpq/pqexpbuffer.c

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,7 @@ enlargePQExpBuffer(PQExpBuffer str, size_t needed)
233233
void
234234
printfPQExpBuffer(PQExpBuffer str, const char *fmt,...)
235235
{
236+
int save_errno = errno;
236237
va_list args;
237238
bool done;
238239

@@ -244,6 +245,7 @@ printfPQExpBuffer(PQExpBuffer str, const char *fmt,...)
244245
/* Loop in case we have to retry after enlarging the buffer. */
245246
do
246247
{
248+
errno = save_errno;
247249
va_start(args, fmt);
248250
done = appendPQExpBufferVA(str, fmt, args);
249251
va_end(args);
@@ -261,6 +263,7 @@ printfPQExpBuffer(PQExpBuffer str, const char *fmt,...)
261263
void
262264
appendPQExpBuffer(PQExpBuffer str, const char *fmt,...)
263265
{
266+
int save_errno = errno;
264267
va_list args;
265268
bool done;
266269

@@ -270,6 +273,7 @@ appendPQExpBuffer(PQExpBuffer str, const char *fmt,...)
270273
/* Loop in case we have to retry after enlarging the buffer. */
271274
do
272275
{
276+
errno = save_errno;
273277
va_start(args, fmt);
274278
done = appendPQExpBufferVA(str, fmt, args);
275279
va_end(args);
@@ -281,6 +285,9 @@ appendPQExpBuffer(PQExpBuffer str, const char *fmt,...)
281285
* Shared guts of printfPQExpBuffer/appendPQExpBuffer.
282286
* Attempt to format data and append it to str. Returns true if done
283287
* (either successful or hard failure), false if need to retry.
288+
*
289+
* Caution: callers must be sure to preserve their entry-time errno
290+
* when looping, in case the fmt contains "%m".
284291
*/
285292
static bool
286293
appendPQExpBufferVA(PQExpBuffer str, const char *fmt, va_list args)

‎src/pl/plpython/plpy_elog.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ static bool set_string_attr(PyObject *obj, char *attrname, char *str);
4646
void
4747
PLy_elog_impl(int elevel, const char *fmt,...)
4848
{
49+
int save_errno = errno;
4950
char *xmsg;
5051
char *tbmsg;
5152
int tb_depth;
@@ -96,6 +97,7 @@ PLy_elog_impl(int elevel, const char *fmt,...)
9697
va_list ap;
9798
int needed;
9899

100+
errno = save_errno;
99101
va_start(ap, fmt);
100102
needed = appendStringInfoVA(&emsg, dgettext(TEXTDOMAIN, fmt), ap);
101103
va_end(ap);

‎src/port/snprintf.c

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,14 @@
6464
*
6565
* 5. Space and '#' flags are not implemented.
6666
*
67+
* In addition, we support some extensions over C99:
68+
*
69+
* 1. Argument order control through "%n$" and "*n$", as required by POSIX.
70+
*
71+
* 2. "%m" expands to the value of strerror(errno), where errno is the
72+
* value that variable had at the start of the call. This is a glibc
73+
* extension, but a very useful one.
74+
*
6775
*
6876
* Historically the result values of sprintf/snprintf varied across platforms.
6977
* This implementation now follows the C99 standard:
@@ -155,6 +163,13 @@ static void flushbuffer(PrintfTarget *target);
155163
static void dopr(PrintfTarget *target, const char *format, va_list args);
156164

157165

166+
/*
167+
* Externally visible entry points.
168+
*
169+
* All of these are just wrappers around dopr(). Note it's essential that
170+
* they not change the value of "errno" before reaching dopr().
171+
*/
172+
158173
int
159174
pg_vsnprintf(char *str, size_t count, const char *fmt, va_list args)
160175
{
@@ -315,11 +330,12 @@ static void trailing_pad(int *padlen, PrintfTarget *target);
315330

316331

317332
/*
318-
* dopr(): poor man's version of doprintf
333+
* dopr(): the guts of *printf for all cases.
319334
*/
320335
static void
321336
dopr(PrintfTarget *target, const char *format, va_list args)
322337
{
338+
int save_errno = errno;
323339
const char *format_start = format;
324340
int ch;
325341
bool have_dollar;
@@ -497,6 +513,7 @@ dopr(PrintfTarget *target, const char *format, va_list args)
497513
else
498514
have_non_dollar = true;
499515
break;
516+
case 'm':
500517
case '%':
501518
break;
502519
}
@@ -802,6 +819,15 @@ dopr(PrintfTarget *target, const char *format, va_list args)
802819
precision, pointflag,
803820
target);
804821
break;
822+
case 'm':
823+
{
824+
char errbuf[PG_STRERROR_R_BUFLEN];
825+
const char *errm = strerror_r(save_errno,
826+
errbuf, sizeof(errbuf));
827+
828+
dostr(errm, strlen(errm), target);
829+
}
830+
break;
805831
case '%':
806832
dopr_outch('%', target);
807833
break;

0 commit comments

Comments
 (0)