NotificationsYou must be signed in to change notification settings
Fork6
Star31

Commit3779ac6

committed

Make printf("%s", NULL) print "(null)" instead of crashing.

We previously took a hard-line attitude that callers should never printa null string pointer, and doing so is worthy of an assertion failureor crash. However, we've long since flushed out any easy-to-find bugsof that nature. What remains is a lot of code that perhaps could failthat way in hard-to-reach corner cases. For example, in something assimple as ereport(ERROR, (errcode(ERRCODE_UNDEFINED_OBJECT), errmsg("constraint \"%s\" for table \"%s\" does not exist", conname, get_rel_name(relid))));one must wonder whether it's completely guaranteed that get_rel_namecannot return NULL in this context. If such a situation did occur,the existing policy converts what might be a pretty minor bug intoa server crash condition. This is not good for robustness.Hence, let's follow the lead of glibc and print "(null)" insteadof failing. We should, of course, still consider it a bug if thatbehavior is reachable in ordinary use; but crashing seems lessdesirable than not crashing.This fix works across-the-board in v12 and up, where we always usesrc/port/snprintf.c. Before that, on most platforms we're at the mercyof the local libc, but it appears that Solaris 10 is the only supportedplatform where we'd still get a crash. Most other platforms such as*BSD, macOS, and Solaris 11 have adopted glibc's behavior at somepoint. (AIX and HPUX just print "" not "(null)", but that's closeenough.) I've not checked what Windows' native printf would do, butit doesn't matter because we've long used snprintf.c on that platform.In v12 and up, also const-ify related code so that we're not castingaway const on the constant string. This is just neatnik-ism, sincenext to no compilers will warn about that.Discussion:https://postgr.es/m/17098-b960f3616c861f83@postgresql.org

1 parent76fa3db commit3779ac6Copy full SHA for 3779ac6

File tree

1 file changed

-7

lines changed

src/port
- snprintf.c

1 file changed

-7

lines changed

`‎src/port/snprintf.c`

Lines changed: 9 additions & 7 deletions

Original file line number	Diff line number	Diff line change
`@@ -320,7 +320,7 @@ static bool find_arguments(const char *format, va_list args,`
`320`	`320`	`PrintfArgValue*argvalues);`
`321`	`321`	`staticvoidfmtstr(constchar*value,intleftjust,intminlen,intmaxwidth,`
`322`	`322`	`intpointflag,PrintfTarget*target);`
`323`		`-staticvoidfmtptr(voidvalue,PrintfTargettarget);`
	`323`	`+staticvoidfmtptr(constvoidvalue,PrintfTargettarget);`
`324`	`324`	`staticvoidfmtint(long longvalue,chartype,intforcesign,`
`325`	`325`	`intleftjust,intminlen,intzpad,intprecision,intpointflag,`
`326`	`326`	`PrintfTarget*target);`
`@@ -394,7 +394,7 @@ dopr(PrintfTarget target, const char format, va_list args)`
`394`	`394`	`intcvalue;`
`395`	`395`	`long longnumvalue;`
`396`	`396`	`doublefvalue;`
`397`		`-char*strvalue;`
	`397`	`+constchar*strvalue;`
`398`	`398`	`PrintfArgValueargvalues[PG_NL_ARGMAX+1];`
`399`	`399`
`400`	`400`	`/*`
`@@ -439,7 +439,8 @@ dopr(PrintfTarget target, const char format, va_list args)`
`439`	`439`	`{`
`440`	`440`	`format++;`
`441`	`441`	`strvalue=va_arg(args,char*);`
`442`		`-Assert(strvalue!=NULL);`
	`442`	`+if (strvalue==NULL)`
	`443`	`+strvalue="(null)";`
`443`	`444`	`dostr(strvalue,strlen(strvalue),target);`
`444`	`445`	`if (target->failed)`
`445`	`446`	`break;`
`@@ -670,8 +671,9 @@ dopr(PrintfTarget target, const char format, va_list args)`
`670`	`671`	`strvalue=argvalues[fmtpos].cptr;`
`671`	`672`	`else`
`672`	`673`	`strvalue=va_arg(args,char*);`
`673`		`-/* Whine if someone tries to print a NULL string */`
`674`		`-Assert(strvalue!=NULL);`
	`674`	`+/* If string is NULL, silently substitute "(null)" */`
	`675`	`+if (strvalue==NULL)`
	`676`	`+strvalue="(null)";`
`675`	`677`	`fmtstr(strvalue,leftjust,fieldwidth,precision,pointflag,`
`676`	`678`	`target);`
`677`	`679`	`break;`
`@@ -681,7 +683,7 @@ dopr(PrintfTarget target, const char format, va_list args)`
`681`	`683`	`strvalue=argvalues[fmtpos].cptr;`
`682`	`684`	`else`
`683`	`685`	`strvalue=va_arg(args,char*);`
`684`		`-fmtptr((void*)strvalue,target);`
	`686`	`+fmtptr((constvoid*)strvalue,target);`
`685`	`687`	`break;`
`686`	`688`	`case'e':`
`687`	`689`	`case'E':`
`@@ -995,7 +997,7 @@ fmtstr(const char *value, int leftjust, int minlen, int maxwidth,`
`995`	`997`	`}`
`996`	`998`
`997`	`999`	`staticvoid`
`998`		`-fmtptr(voidvalue,PrintfTargettarget)`
	`1000`	`+fmtptr(constvoidvalue,PrintfTargettarget)`
`999`	`1001`	`{`
`1000`	`1002`	`intvallen;`
`1001`	`1003`	`charconvert[64];`

0 commit comments

Comments

(0)

Movatterモバイル変換

Navigation Menu

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Commit3779ac6

File tree

1 file changed

1 file changed

`‎src/port/snprintf.c`

0 commit comments