pgsql: Cosmetic improvements in new config_info code.
Cosmetic improvements in new config_info code.
Coverity griped about use of unchecked strcpy() into a local variable.
There's unlikely to be any actual bug there, since no caller would be
passing a path longer than MAXPGPATH, but nonetheless use of strlcpy()
seems preferable.
While at it, get rid of unmaintainable separation between list of
field names and list of field values in favor of initializing them
in parallel. And we might as well declare get_configdata()'s path
argument as const char *, even though no current caller needs that.
Branch
------
master
Details
-------
http://git.postgresql.org/pg/commitdiff/c7a1c5a6b6aa4bbc2c9619edc94368fccc1c8c8e
Modified Files
--------------
src/common/config_info.c | 176 +++++++++++++++++++--------------------
src/include/common/config_info.h | 2 +-
2 files changed, 89 insertions(+), 89 deletions(-)
--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers
On 02/21/2016 08:38 AM, Tom Lane wrote:
Coverity griped about use of unchecked strcpy() into a local variable.
There's unlikely to be any actual bug there, since no caller would be
passing a path longer than MAXPGPATH, but nonetheless use of strlcpy()
seems preferable.
FWIW, strcpy() was being used in src/bin/pg_config/pg_config.c that I
started with -- does that mean we are not getting Coverity coverage of
src/bin?
While at it, get rid of unmaintainable separation between list of
field names and list of field values in favor of initializing them
in parallel.
I waffled back and forth about doing something similar and apparently
landed on the wrong side of it :-). I'll take a similar tack with the
controldata patch that I'm trying to get over the finish line.
Joe
--
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development
Joe, all,
* Joe Conway (mail@joeconway.com) wrote:
On 02/21/2016 08:38 AM, Tom Lane wrote:
Coverity griped about use of unchecked strcpy() into a local variable.
There's unlikely to be any actual bug there, since no caller would be
passing a path longer than MAXPGPATH, but nonetheless use of strlcpy()
seems preferable.FWIW, strcpy() was being used in src/bin/pg_config/pg_config.c that I
started with -- does that mean we are not getting Coverity coverage of
src/bin?
Coverity does run against src/bin also. It's possible this was
identified as an issue in pg_config.c, but, as Tom notes, it may not be
an actual bug and might have been marked as a non-bug in Coverity.
Thanks!
Stephen
Stephen Frost <sfrost@snowman.net> writes:
* Joe Conway (mail@joeconway.com) wrote:
FWIW, strcpy() was being used in src/bin/pg_config/pg_config.c that I
started with -- does that mean we are not getting Coverity coverage of
src/bin?
Coverity does run against src/bin also. It's possible this was
identified as an issue in pg_config.c, but, as Tom notes, it may not be
an actual bug and might have been marked as a non-bug in Coverity.
It looks to me like the previous coding was
static char mypath[MAXPGPATH];
...
char path[MAXPGPATH];
...
strcpy(path, mypath);
so Coverity probably didn't complain because it could see that the source
was also a buffer of size MAXPGPATH. With the new arrangement it was
probably using an assumption that get_configdata() could be called with
any length of string.
I am not sure how much cross-file analysis Coverity does --- it seems to
do some, but in other cases it acts like it doesn't know anything about
the call sites. It's possible that it did know that the existing callers
all use MAXPGPATH-sized buffers, but whined anyway on the not-unreasonable
grounds that global functions should be prepared for new callers.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers