From 1fcf7e8be5aa38a02d7bc88ea8df66758f81b8d6 Mon Sep 17 00:00:00 2001 From: Georgios Kokolatos Date: Tue, 12 Mar 2019 15:08:59 +0000 Subject: [PATCH] Make unsafe greenplum specific function linenumber_atoi() safer The function was using an anti-pattern where an argument is a static length char array. Array arguments in C don't really exist but compilers accept them and without any 'const' or 'static' decorators they don't always emit a warning. The proposed patch only fixes the unsafeness of the function declaration but it does not address the usefulness of it. Reviewed-by: Daniel Gustafsson --- src/backend/access/external/fileam.c | 14 +++++++------- src/backend/commands/copy.c | 12 ++++++------ src/include/access/fileam.h | 2 +- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/backend/access/external/fileam.c b/src/backend/access/external/fileam.c index 4ead0ea718..f1fda76ca9 100644 --- a/src/backend/access/external/fileam.c +++ b/src/backend/access/external/fileam.c @@ -1473,7 +1473,7 @@ external_scan_error_callback(void *arg) errcontext("External table %s, line %s of %s, column %s", cstate->cur_relname, - linenumber_atoi(buffer, cstate->cur_lineno), + linenumber_atoi(buffer, sizeof(buffer), cstate->cur_lineno), scan->fs_uri, cstate->cur_attname); } @@ -1489,7 +1489,7 @@ external_scan_error_callback(void *arg) errcontext("External table %s, line %s of %s: \"%s\"", cstate->cur_relname, - linenumber_atoi(buffer, cstate->cur_lineno), + linenumber_atoi(buffer, sizeof(buffer), cstate->cur_lineno), scan->fs_uri, line_buf); pfree(line_buf); } @@ -1510,7 +1510,7 @@ external_scan_error_callback(void *arg) if (cstate->cur_lineno > 0) errcontext("External table %s, line %s of file %s", cstate->cur_relname, - linenumber_atoi(buffer, cstate->cur_lineno), + linenumber_atoi(buffer, sizeof(buffer), cstate->cur_lineno), scan->fs_uri); else errcontext("External table %s, file %s", @@ -1600,12 +1600,12 @@ justifyDatabuf(StringInfo buf) } char * -linenumber_atoi(char buffer[20], int64 linenumber) +linenumber_atoi(char *buffer, size_t bufsz, int64 linenumber) { if (linenumber < 0) - return "N/A"; - - snprintf(buffer, 20, INT64_FORMAT, linenumber); + snprintf(buffer, bufsz, "%s", "N/A"); + else + snprintf(buffer, bufsz, INT64_FORMAT, linenumber); return buffer; } diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c index 933dbc6974..f0f35c4f41 100644 --- a/src/backend/commands/copy.c +++ b/src/backend/commands/copy.c @@ -3274,12 +3274,12 @@ CopyFromErrorCallback(void *arg) if (cstate->cur_attname) errcontext("COPY %s, line %s, column %s", cstate->cur_relname, - linenumber_atoi(buffer, cstate->cur_lineno), + linenumber_atoi(buffer, sizeof(buffer), cstate->cur_lineno), cstate->cur_attname); else errcontext("COPY %s, line %s", cstate->cur_relname, - linenumber_atoi(buffer, cstate->cur_lineno)); + linenumber_atoi(buffer, sizeof(buffer), cstate->cur_lineno)); } else { @@ -3291,7 +3291,7 @@ CopyFromErrorCallback(void *arg) attval = limit_printout_length(cstate->cur_attval); errcontext("COPY %s, line %s, column %s: \"%s\"", cstate->cur_relname, - linenumber_atoi(buffer, cstate->cur_lineno), + linenumber_atoi(buffer, sizeof(buffer), cstate->cur_lineno), cstate->cur_attname, attval); pfree(attval); } @@ -3300,7 +3300,7 @@ CopyFromErrorCallback(void *arg) /* error is relevant to a particular column, value is NULL */ errcontext("COPY %s, line %s, column %s: null input", cstate->cur_relname, - linenumber_atoi(buffer, cstate->cur_lineno), + linenumber_atoi(buffer, sizeof(buffer), cstate->cur_lineno), cstate->cur_attname); } else @@ -3323,7 +3323,7 @@ CopyFromErrorCallback(void *arg) lineval = limit_printout_length(cstate->line_buf.data); errcontext("COPY %s, line %s: \"%s\"", cstate->cur_relname, - linenumber_atoi(buffer, cstate->cur_lineno), + linenumber_atoi(buffer, sizeof(buffer), cstate->cur_lineno), lineval); pfree(lineval); } @@ -3339,7 +3339,7 @@ CopyFromErrorCallback(void *arg) */ errcontext("COPY %s, line %s", cstate->cur_relname, - linenumber_atoi(buffer, cstate->cur_lineno)); + linenumber_atoi(buffer, sizeof(buffer), cstate->cur_lineno)); } } } diff --git a/src/include/access/fileam.h b/src/include/access/fileam.h index c3ae4adb44..242220d8bb 100644 --- a/src/include/access/fileam.h +++ b/src/include/access/fileam.h @@ -85,7 +85,7 @@ extern ExternalInsertDesc external_insert_init(Relation rel); extern Oid external_insert(ExternalInsertDesc extInsertDesc, HeapTuple instup); extern void external_insert_finish(ExternalInsertDesc extInsertDesc); extern void external_set_env_vars(extvar_t *extvar, char *uri, bool csv, char *escape, char *quote, bool header, uint32 scancounter); -extern char *linenumber_atoi(char buffer[20], int64 linenumber); +extern char *linenumber_atoi(char *buffer, size_t bufsz, int64 linenumber); /* prototypes for functions in url_execute.c */ extern int popen_with_stderr(int *rwepipe, const char *exe, bool forwrite); -- GitLab