Fabien COELHO <coelho@cri.ensmp.fr> writes:
> There seems to be other instances as well
Thanks! I made the same modification of src/interfaces/ecpg/preproc/ecpg.c, but
in other cases it's either not a problem (as with src/bin/psql/startup.c or
src/timezone/zic.c) or the solution is too complex to be added to the current
patch: e.g. these tools in src/bin/scripts use handle_help_version_opts function
from src/bin/scripts/common.c which cannot be refactored so straightforward.
It's possible to remove it and modify each tool to parse the --help and
--version args for themselves but I would not include it in the same patch as
it's already not too short for a "fix" patch and these changes are better to be
discussed separately IMHO. Do you think the handle_help_version_opts function
should be removed and these args should be parsed in each src/bin/scripts app?
There are several cases where separate comparisons of argv[1] are made to detect
"--help" or "--version" before non-root user is enforced (to make it possible to
the root user to check the version) e.g. src/bin/pg_upgrade/option.c — in this
case I left this comparisons untouched while expanding the switch statement of
getopt_long, so non-root user sees the correct behavior and root still sees
"cannot be run as root" error when trying # pg_upgrade -v --help. The
alternative is to wrap these argv[...] comparisons in a for statement to
iterate through all the arguments. Another option is to enforce non-root after
getopt_long parsing but it's harder to be sure that the patch does not alter
the apps behavior unexpected way.
There are also the few apps when getopt is used instead of getopt_long, so I
decided not to fix these in the current patch because it's not so obvious. It's
possible either to replace getopt with getopt_long (and add long options, which
may be nice) or wrap --help/--version parsing in a for loop.
--
Andrei