pgsql: StrNCpy -> strlcpy (not complete)

Woh. Peter, you realize one of the reasons we use StrNCpy as a macro is
for performance. I don't see strlcpy as a macro. Are you going to
change all call locations to strlcpy? If so, have you measured the
performance impact?

---------------------------------------------------------------------------

Peter Eisentraut wrote:

Log Message:
-----------
StrNCpy -> strlcpy (not complete)

Modified Files:
--------------
pgsql/src/backend/bootstrap:
bootstrap.c (r1.229 -> r1.230)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/backend/bootstrap/bootstrap.c.diff?r1=1.229&r2=1.230)
pgsql/src/backend/libpq:
crypt.c (r1.72 -> r1.73)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/backend/libpq/crypt.c.diff?r1=1.72&r2=1.73)
hba.c (r1.159 -> r1.160)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/backend/libpq/hba.c.diff?r1=1.159&r2=1.160)
ip.c (r1.39 -> r1.40)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/backend/libpq/ip.c.diff?r1=1.39&r2=1.40)
pgsql/src/backend/nodes:
print.c (r1.83 -> r1.84)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/backend/nodes/print.c.diff?r1=1.83&r2=1.84)
pgsql/src/backend/postmaster:
pgarch.c (r1.28 -> r1.29)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/backend/postmaster/pgarch.c.diff?r1=1.28&r2=1.29)
postmaster.c (r1.518 -> r1.519)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/backend/postmaster/postmaster.c.diff?r1=1.518&r2=1.519)
pgsql/src/backend/tcop:
postgres.c (r1.521 -> r1.522)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/backend/tcop/postgres.c.diff?r1=1.521&r2=1.522)
pgsql/src/backend/utils/misc:
guc-file.l (r1.46 -> r1.47)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/backend/utils/misc/guc-file.l.diff?r1=1.46&r2=1.47)
pgsql/src/bin/initdb:
initdb.c (r1.131 -> r1.132)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/bin/initdb/initdb.c.diff?r1=1.131&r2=1.132)
pgsql/src/bin/pg_ctl:
pg_ctl.c (r1.77 -> r1.78)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/bin/pg_ctl/pg_ctl.c.diff?r1=1.77&r2=1.78)
pgsql/src/bin/pg_dump:
pg_dumpall.c (r1.89 -> r1.90)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/bin/pg_dump/pg_dumpall.c.diff?r1=1.89&r2=1.90)
pgsql/src/bin/pg_resetxlog:
pg_resetxlog.c (r1.56 -> r1.57)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/bin/pg_resetxlog/pg_resetxlog.c.diff?r1=1.56&r2=1.57)
pgsql/src/interfaces/libpq:
fe-auth.c (r1.122 -> r1.123)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/interfaces/libpq/fe-auth.c.diff?r1=1.122&r2=1.123)
fe-connect.c (r1.342 -> r1.343)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/interfaces/libpq/fe-connect.c.diff?r1=1.342&r2=1.343)
pgsql/src/timezone:
pgtz.c (r1.49 -> r1.50)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/timezone/pgtz.c.diff?r1=1.49&r2=1.50)

---------------------------(end of broadcast)---------------------------
TIP 6: explain analyze is your friend

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

On Sat, Feb 10, 2007 at 09:21:04PM -0500, Bruce Momjian wrote:

Woh. Peter, you realize one of the reasons we use StrNCpy as a macro is
for performance. I don't see strlcpy as a macro. Are you going to
change all call locations to strlcpy? If so, have you measured the
performance impact?

I think we had this discussion already. strncpy() copies N bytes,
whereas strlcpy() copies only as many bytes as necessary. For short
strings with larger buffers, strlcpy() wins. It's understood that
in many cases in PostgreSQL, the expectation is for short strings,
and it is not required for the later bytes to be '\0'.

I assume Peter is only changing the provably good uses? :-)

Cheers,
mark

Peter Eisentraut wrote:

Log Message:
-----------
StrNCpy -> strlcpy (not complete)

Modified Files:
--------------
pgsql/src/backend/bootstrap:
bootstrap.c (r1.229 -> r1.230)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/backend/bootstrap/bootstrap.c.diff?r1=1.229&amp;r2=1.230)
pgsql/src/backend/libpq:
crypt.c (r1.72 -> r1.73)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/backend/libpq/crypt.c.diff?r1=1.72&amp;r2=1.73)
hba.c (r1.159 -> r1.160)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/backend/libpq/hba.c.diff?r1=1.159&amp;r2=1.160)
ip.c (r1.39 -> r1.40)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/backend/libpq/ip.c.diff?r1=1.39&amp;r2=1.40)
pgsql/src/backend/nodes:
print.c (r1.83 -> r1.84)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/backend/nodes/print.c.diff?r1=1.83&amp;r2=1.84)
pgsql/src/backend/postmaster:
pgarch.c (r1.28 -> r1.29)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/backend/postmaster/pgarch.c.diff?r1=1.28&amp;r2=1.29)
postmaster.c (r1.518 -> r1.519)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/backend/postmaster/postmaster.c.diff?r1=1.518&amp;r2=1.519)
pgsql/src/backend/tcop:
postgres.c (r1.521 -> r1.522)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/backend/tcop/postgres.c.diff?r1=1.521&amp;r2=1.522)
pgsql/src/backend/utils/misc:
guc-file.l (r1.46 -> r1.47)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/backend/utils/misc/guc-file.l.diff?r1=1.46&amp;r2=1.47)
pgsql/src/bin/initdb:
initdb.c (r1.131 -> r1.132)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/bin/initdb/initdb.c.diff?r1=1.131&amp;r2=1.132)
pgsql/src/bin/pg_ctl:
pg_ctl.c (r1.77 -> r1.78)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/bin/pg_ctl/pg_ctl.c.diff?r1=1.77&amp;r2=1.78)
pgsql/src/bin/pg_dump:
pg_dumpall.c (r1.89 -> r1.90)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/bin/pg_dump/pg_dumpall.c.diff?r1=1.89&amp;r2=1.90)
pgsql/src/bin/pg_resetxlog:
pg_resetxlog.c (r1.56 -> r1.57)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/bin/pg_resetxlog/pg_resetxlog.c.diff?r1=1.56&amp;r2=1.57)
pgsql/src/interfaces/libpq:
fe-auth.c (r1.122 -> r1.123)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/interfaces/libpq/fe-auth.c.diff?r1=1.122&amp;r2=1.123)
fe-connect.c (r1.342 -> r1.343)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/interfaces/libpq/fe-connect.c.diff?r1=1.342&amp;r2=1.343)
pgsql/src/timezone:
pgtz.c (r1.49 -> r1.50)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/timezone/pgtz.c.diff?r1=1.49&amp;r2=1.50)

---------------------------(end of broadcast)---------------------------
TIP 6: explain analyze is your friend

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

---------------------------(end of broadcast)---------------------------
TIP 3: Have you checked our extensive FAQ?

http://www.postgresql.org/docs/faq

--
mark@mielke.cc / markm@ncf.ca / markm@nortel.com __________________________
. . _ ._ . . .__ . . ._. .__ . . . .__ | Neighbourhood Coder
|\/| |_| |_| |/ |_ |\/| | |_ | |/ |_ |
| | | | | \ | \ |__ . | | .|. |__ |__ | \ |__ | Ottawa, Ontario, Canada

One ring to rule them all, one ring to find them, one ring to bring them all
and in the darkness bind them...

http://mark.mielke.cc/

Bruce Momjian <bruce@momjian.us> writes:

Woh. Peter, you realize one of the reasons we use StrNCpy as a macro is
for performance. I don't see strlcpy as a macro.

Huh? StrNCpy is a wrapper around strncpy(). Do you have reason to
think that strncpy() is especially tightly implemented on most
platforms?

regards, tom lane

mark@mark.mielke.cc wrote:

I think we had this discussion already. strncpy() copies N bytes,
whereas strlcpy() copies only as many bytes as necessary. For short
strings with larger buffers, strlcpy() wins. It's understood that
in many cases in PostgreSQL, the expectation is for short strings,
and it is not required for the later bytes to be '\0'.

You may also speculate that strncpy() is more optimized in some C
libraries than strlcpy(). However, the changed cases are all
uninteresting in terms of performance or fall under the short strings
in long buffers case.

The remaining uses of StrNCpy() are either inner loops which need to be
investigated, or it's not clear whether the zero-filling of strncpy()
is required, or it's in a library were the libpgport linkages needs to
be added.

The main idea here is to get this programming style out because it's
become clear that people are very confused about how to use some of the
other functions correctly.

--
Peter Eisentraut
http://developer.postgresql.org/~petere/

Peter Eisentraut wrote:

mark@mark.mielke.cc wrote:

I think we had this discussion already. strncpy() copies N bytes,
whereas strlcpy() copies only as many bytes as necessary. For short
strings with larger buffers, strlcpy() wins. It's understood that
in many cases in PostgreSQL, the expectation is for short strings,
and it is not required for the later bytes to be '\0'.

You may also speculate that strncpy() is more optimized in some C
libraries than strlcpy(). However, the changed cases are all
uninteresting in terms of performance or fall under the short strings
in long buffers case.

The remaining uses of StrNCpy() are either inner loops which need to be
investigated, or it's not clear whether the zero-filling of strncpy()
is required, or it's in a library were the libpgport linkages needs to
be added.

The main idea here is to get this programming style out because it's
become clear that people are very confused about how to use some of the
other functions correctly.

Sorry, I was confusing this with MemSet. Thanks for the clarification.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +