[pgpool-hackers: 247] Re: Ideas for code cleanup

Bruce Momjian bruce at momjian.us
Wed May 22 22:47:33 JST 2013


On Wed, May 22, 2013 at 06:16:29PM +0900, Tatsuo Ishii wrote:
> >> I have a question: you replaced strcmp to strlen in your patches.
> >> Why do you prefer strlen over strcmp?
> > 
> > Well, I thought testing for a zero-length was clearer than comparing to
> > "".  Another approach is to test if [0] == '\0'.  I had just never seen
> > the "" comparison before, and it looked odd.  I think we should just
> > pick a method and use it consistently, probably based on how the
> > Postgres does it.
> 
> I see these in the PostgreSQL source code:
> find . -name '*.c' -print|xargs grep -i strcmp|grep \"\"
> 
> ./timezone/zic.c:		if (strcmp(cp, "") == 0)
> grep: ./bin/pg_controldata/pg_crc.c: そのようなファイルやディレクトリはありません
> ./bin/initdb/initdb.c:							  (strcmp(authmethodlocal, "trust") == 0 || strcmp(authmethodhost, "trust") == 0) ? AUTHTRUST_WARNING : "");
> ./bin/initdb/initdb.c:	if (strcmp(xlog_dir, "") != 0)
> ./bin/psql/describe.c:						   strcmp(PQgetvalue(res, 0, 8), "") != 0) ?
> ./bin/psql/describe.c:		if (strcmp(PQgetvalue(res, i, 7), "") != 0)
> ./bin/pg_dump/pg_backup_custom.c:		if (AH->fSpec && strcmp(AH->fSpec, "") != 0)
> ./bin/pg_dump/pg_backup_custom.c:		if (AH->fSpec && strcmp(AH->fSpec, "") != 0)
> ./bin/pg_dump/pg_backup_custom.c:	if (AH->fSpec == NULL || strcmp(AH->fSpec, "") == 0)
> ./bin/pg_dump/pg_backup_tar.c:		if (AH->fSpec && strcmp(AH->fSpec, "") != 0)
> ./bin/pg_dump/pg_backup_tar.c:		if (AH->fSpec && strcmp(AH->fSpec, "") != 0)
> ./bin/pg_dump/pg_backup_archiver.c:		(AH->fSpec == NULL || strcmp(AH->fSpec, "") == 0))
> ./bin/pg_dump/pg_backup_archiver.c:	if (strcmp(want, "") == 0)
> ./bin/pg_dump/pg_dump.c:	if (strcmp("", evtinfo->evttags) != 0)
> ./bin/pg_dump/pg_backup_directory.c:	if (!AH->fSpec || strcmp(AH->fSpec, "") == 0)
> ./backend/commands/dbcommands.c:			if (strcmp(encoding_name, "") == 0 ||

I remember now --- what I found confusing was having "" at the first
argument of strcmp():

	if (strcmp("", pool_config->pool_passwd))

I think changing it to:

	if (strcmp(pool_config->pool_passwd, ""))

would be fine, though you might want to use:

	if (strcmp(pool_config->pool_passwd, "") != 0)

so it is clear we are testing for not-equal to "".

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

  + It's impossible for everything to be true. +


More information about the pgpool-hackers mailing list