[m-rev.] for review: implement C version of mmc
Julien Fischer
juliensf at cs.mu.OZ.AU
Mon Oct 3 23:40:55 AEST 2005
On Sat, 1 Oct 2005, Ian MacLarty wrote:
> For review by anyone.
>
> Estimated hours taken: 30
> Branches: main and 0.12
>
> Implement a C version of mmc that can be run on Windows without a unix shell.
>
> The new mmc can also operate in a standalone mode where it determines where it
s/where it/that
> is being run from, allowing it to still work even if the installed compiler is
> moved to a new directory. This mode will be used for the upcoming Windows
> installer.
>
> Add some configure options which turn on the stanalone mode. In the standalone
/which turn on/that enable/
s/stanalone/standalone/
> mode relative paths to the C compiler and linker must also be supplied. These
> are assumed to be packaged with the Mercury compiler in standalone mode. mmc
> will convert the relative paths to absolute paths at runtime and pass these
> to the Mercury compiler.
>
> In non-stanalone mode the C version of mmc behaves exactly the same as the
> shell script version. The C version is now the version which is installed to
> the bin directory, however the shell version is still used in the compiler
> build process and for bootchecking.
>
Could we delay making the C version the default? I think it should
be tested a lot more extensively first.
> Fix some unrelated C compiler warnings (these changes are very short).
>
> README.MS-Windows:
> Add a reference to README.Standalone.
>
> README.Standalone:
> Add a description of how to package the Mercury compiler and C
> compiler up in a standalone package.
> Include an example of how to package the Mercury compiler up with
Delete up.
> MinGW, so it is independent of Cygwin.
>
> configure.in:
> Add options to enable the standalone version of mmc and to give
> relative paths for the C compiler and the linker commands that generate
> an executable and a shared library.
What's the relationship between the C compiler used to build the Mercury
compiler and the one used by the standalone version?
> If standalone mode is enabled then
> all three relative paths must be present. If standalone mode is not
> enabled, then the relative paths are ignored.
>
> Set the configure variable that determines whether the
> MACOSX_DEPLOYMENT_TARGET environment variable should be set, to 1 or 0,
> instead of setting it to the actual shell command which sets the
> environment variable. This is done so that the configure variable
> can be checked in both the C and sh versions of mmc.
>
> Build util/mmc.c.
>
> library/dir.m:
> library/io.m:
> Fix two C compiler warnings.
>
> scripts/Mmakefile:
> Do not install the sh version of mmc.
>
> scripts/ml.in:
> scripts/mmake.in:
> scripts/mmc.in:
> Set MACOSX_DEPLOYMENT_TARGET if necessary.
>
> util/Mmakefile:
> Build the C version of mmc. It has different rules to the
> other C files in util, because it links in path_util.
>
> util/mmc.c.in:
> The C version of mmc.
>
> util/path_util.c:
> util/path_util.h:
> Utility functions used by mmc to determine where it is being executed
> from.
>
> Index: README.MS-Windows
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/README.MS-Windows,v
> retrieving revision 1.19
> diff -u -r1.19 README.MS-Windows
> --- README.MS-Windows 30 May 2004 22:59:33 -0000 1.19
> +++ README.MS-Windows 1 Oct 2005 08:30:41 -0000
> @@ -175,4 +175,9 @@
> mdemangle.exe (in <prefix>/bin)
> mkinit.exe (in <prefix>/bin)
>
> +The Mercury compiler built using the above process will still depend on cygwin,
> +but the executables it generates will not. It is also possible to build a
> +version of the compiler which is completely independent of Cygwin. For details
s/which/that/
> +on how to build such a compiler see README.Standalone.
> +
> -----------------------------------------------------------------------------
> Index: README.Standalone
> ===================================================================
> RCS file: README.Standalone
> diff -N README.Standalone
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ README.Standalone 1 Oct 2005 08:27:44 -0000
> @@ -0,0 +1,94 @@
> +-----------------------------------------------------------------------------
> +
> +INTRODUCTION
> +
> +This file describes how to build a version of the Mercury compiler that has
> +a C compiler and linker packaged with it and so doesn't depend on any external
> +programs.
Doesn't it still need the sort utility in order to use fact tables?
> The directory containing the Mercury and C compilers can be moved
> +around and mmc will still work (although at this stage mmake, mdb, etc will
> +not). This property makes the standalone version of the compiler ideal for
> +distribution in a binary package.
You should make it clear that you are referring to the C version here
not the shell version.
> +
> +-----------------------------------------------------------------------------
> +
> +OVERVIEW
> +
> +mmc is a wrapper which calls the real Mercury compiler executable,
s/which/that/
> +mercury_compile. mmc can operate in one of two modes: standalone mode or
> +non-standalone mode.
> +
> +In non-standalone mode the paths to the Mercury compiler, the C compiler and
> +the linker are all resolved at configuration time. This means that once the
> +compiler is installed, it cannot be moved without reconfiguring.
> +
> +In standalone mode relative paths to the C compiler and linker are supplied
> +at configuration time. mmc then resolves the absolute paths to the C compiler
> +and linker each time it is run and passes these to the Mercury compiler.
> +This means that once the Mercury compiler is installed, the installation
> +directory can be moved and mmc will still work as long as the C compiler
> +and linker are still in the same relative path.
> +
I suspect that shifting the libraries around on OS X
is going to cause problems - I guess its not too much of a problem
because we can always hardcode gcc3.3 as the C compiler on OS 10.3 and
10.4.
> +-----------------------------------------------------------------------------
> +
> +CONFIGURATION
> +
> +To enable mmc to operate in standalone mode, four options must be given to
> +the configure script.
> +
> +1. --enable-standalone
> + This causes mmc to be compiled in standalone mode.
> +2. --with-relative-cc=<relative path to C compiler>
> + This specifies the relative command to invoke the C compiler. The
> + command should be relative to the installation directory (i.e. the
> + directory given with the --prefix configure option).
What's the relationship between this option and the with-cc option?
> +3. --with-relative-link-exec-cmd=<relative path to link command>
> + This specifies the relative path to the command to invoke the linker to
> + generate an executable.
> +4. --with-relative-link-shared-cmd=<relative path to link command>
> + This specifies the relative path to the command to invoke the linker to
> + generate a shared library.
> +
> +-----------------------------------------------------------------------------
> +
> +BUILDING
> +
> +After configuration the compiler can be built and installed as usual
> +(using "make" and "make install" with the source distribution or
> +"mmake" and "mmake install" with the CVS version).
> +
> +After installation the C compiler and linker as well as all other files
> +needed by the C compiler and linker should be copied to the correct
> +location relative to the installed Mercury compiler directory. All these
> +files can then be packaged up and moved around and mmc will continue to
> +work as long as it is in the PATH.
> +
Except on if Microsoft managment console is in the path before it :-)
(You may want to add a pointer to README.MS-Windows or wherever that is
discussed.)
> +-----------------------------------------------------------------------------
> +
> +MINGW EXAMPLE
> +
> +As an example here is how to build a standalone Windows version of the Mercury
> +compiler that comes packaged with MinGW and so doesn't need any other
> +external programs.
> +
> +First you'll need to download and install Cygwin and MinGW.
> +Next download the Mercury source distribution and unpack it in Cygwin.
> +In Cygwin, cd into the source distribution directory and do:
> +
> +./configure --with-cc=/cygdrive/c/mingw/bin/gcc.exe --prefix=<install_dir> --enable-standalone --with-relative-cc=mingw\\\\bin\\\\gcc.exe --with-relative-link-exec-cmd=mingw\\\\bin\\\\gcc.exe --with-relative-link-shared-cmd=mingw\\\\bin\\\\gcc.exe
> +
> +make
> +
> +make install
> +
> +The above assumes that MinGW is installed in c:\mingw.
> +Note that the \ characters must be double escaped: once for the shell and
> +once for the C program they will end up in.
> +<install_dir> above is a directory to install the Mercury compiler to
> +(the directory will be created if it doesn't exist).
> +
> +After "make install" has finished copy the c:\mingw directory into
> +<install_dir>.
> +
> +<install_dir> can now be packaged up and copied to another Windows machine
> +and mmc should work from the Windows command prompt as long as mmc is in the
s/should/will/
> +PATH. Cygwin is not needed on the machine the package is deployed to.
> Index: configure.in
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/configure.in,v
> retrieving revision 1.434
> diff -u -r1.434 configure.in
> --- configure.in 23 Sep 2005 06:39:46 -0000 1.434
> +++ configure.in 28 Sep 2005 11:13:27 -0000
> @@ -314,6 +314,72 @@
> CC="$mercury_cv_with_cc"
> ;;
> esac
> +
> +#-----------------------------------------------------------------------------#
> +# Check if we should enable the standalone version of mmc.
> +AC_ARG_ENABLE(standalone,
> +[ --enable-standalone Enable the standalone version of mmc. If this
> + option is enabled then the --with-relative-cc,
> + --with-relative-link-exec-cmd and
> + --with-relative-link-shared-cmd options must also
> + be set.],
> +mercury_cv_enable_standalone="yes", mercury_cv_enable_standalone="no")
> +AC_ARG_WITH(relative-cc,
> +[ --with-relative-cc Specify the path to the C compiler relative to
> + the installed Mercury directory. The given C
> + compiler is only used if the --enable-standalone
> + option is given.],
> +mercury_cv_with_relative_cc="$withval", mercury_cv_with_relative_cc="")
> +AC_ARG_WITH(relative-link-exec-cmd,
> +[ --with-relative-link-exec-cmd
> + Specify the command to link object files into an
> + executable. The command is relative to the Mercury
> + installed directory. The specified link command
> + will only be used if the --enable-standalone option
> + is also given.],
> +mercury_cv_with_relative_link_exec_cmd="$withval",
> +mercury_cv_with_relative_link_exec_cmd="")
> +AC_ARG_WITH(relative-link-shared-cmd,
> +[ --with-relative-link-shared-cmd
> + Specify the command to link object files into a
> + shared library. The command is relative to the
> + Mercury installed directory. The specified link
s/installed/installation/ here and above.
> + command will only be used if the --enable-standalone
> + option is also given.],
> +mercury_cv_with_relative_link_shared_cmd="$withval",
> +mercury_cv_with_relative_link_shared_cmd="")
> +case "$mercury_cv_enable_standalone" in
> + yes) case "$mercury_cv_with_relative_cc" in
> + "") AC_MSG_ERROR(--with-relative-cc is required if --enable-standalone is given)
> + exit 1
> + ;;
> + *) C_COMPILER_RELATIVE_PATH="$mercury_cv_with_relative_cc"
> + AC_SUBST(C_COMPILER_RELATIVE_PATH)
> + ;;
> + esac
> + case "$mercury_cv_with_relative_link_exec_cmd" in
> + "") AC_MSG_ERROR(--with-relative-link-exec-cmd is required if --enable-standalone is given)
> + exit 1
> + ;;
> + *) LINK_EXEC_CMD_RELATIVE_PATH="$mercury_cv_with_relative_link_exec_cmd"
> + AC_SUBST(LINK_EXEC_CMD_RELATIVE_PATH)
> + ;;
> + esac
> + case "$mercury_cv_with_relative_link_shared_cmd" in
> + "") AC_MSG_ERROR(--with-relative-link-shared-cmd is required if --enable-standalone is given)
> + exit 1
> + ;;
> + *) LINK_SHARED_LIB_CMD_RELATIVE_PATH="$mercury_cv_with_relative_link_shared_cmd"
> + AC_SUBST(LINK_SHARED_LIB_CMD_RELATIVE_PATH)
> + ;;
> + esac
> + MMC_STANDALONE=1
> + ;;
> + no) MMC_STANDALONE=0
> + ;;
> +esac
> +AC_SUBST(MMC_STANDALONE)
> +
> #-----------------------------------------------------------------------------#
> #
> # Find the GCC source code
> @@ -3143,6 +3209,13 @@
> ERROR_UNDEFINED=""
> DEFAULT_LINKAGE="shared"
>
> +# On Darwin the MACOSX_DEPLOYMENT_TARGET environment variable needs to be set
> +# when linking with two level namespaces so we can use the `-undefined
> +# dynamic_lookup' option. If necessary the configure script will set the
> +# following variable to 1 which will cause mmc to set the
> +# MACOSX_DEPLOYMENT_TARGET environment variable.
> +SET_MACOSX_DEPLOYMENT_TARGET=0
> +
> case "$host" in
> i*86-*-linux|i*86-*-linux-gnu)
> case $ac_cv_prog_gcc in
> @@ -3391,14 +3464,7 @@
> -a 7 -le;
> then
> AC_MSG_RESULT(yes)
> - # The MACOSX_DEPLOYMENT_TARGET environment variable
> - # needs to be set when linking with two level
> - # namespaces so we can use the
> - # `-undefined dynamic_lookup' option.
> - SET_MACOSX_DEPLOYMENT_TARGET="\
> - MACOSX_DEPLOYMENT_TARGET=10.3; \
> - export MACOSX_DEPLOYMENT_TARGET"
> - AC_SUBST(SET_MACOSX_DEPLOYMENT_TARGET)
> + SET_MACOSX_DEPLOYMENT_TARGET=1
> LINK_SHARED_OBJ="$GCC_PROG -multiply_defined suppress \
> -dynamiclib -single_module"
> LINK_SHARED_OBJ_SH="$GCC_PROG -multiply_defined \
> @@ -3431,6 +3497,7 @@
> AC_MSG_RESULT(no)
> ;;
> esac
> +AC_SUBST(SET_MACOSX_DEPLOYMENT_TARGET)
>
> #-----------------------------------------------------------------------------#
> # Note that changes here may require changes in scripts/mgnuc.in.
> @@ -4149,7 +4216,7 @@
> # if you add new .in files to either of those you *must* update
> # scripts/mercury_config.in.
> #
> -AC_OUTPUT(Mmake.common scripts/Mmake.vars scripts/mmc
> +AC_OUTPUT(Mmake.common scripts/Mmake.vars scripts/mmc util/mmc.c
> scripts/mercury.bat scripts/mprof scripts/mercury_update_interface
> scripts/mgnuc scripts/parse_ml_options.sh-subr scripts/ml
> scripts/c2init scripts/mmake scripts/mdb scripts/mdbrc scripts/mdprof
Does this require any changes to scripts/mercury_config.in?
...
> #-----------------------------------------------------------------------------#
> Index: util/mmc.c.in
> ===================================================================
> RCS file: util/mmc.c.in
> diff -N util/mmc.c.in
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ util/mmc.c.in 1 Oct 2005 08:34:22 -0000
> @@ -0,0 +1,149 @@
> +/*
> +** vim: ts=4 sw=4 expandtab
> +*/
> +/*
> +** Copyright (C) 2005 The University of Melbourne.
> +** This file may only be copied under the terms of the GNU Library General
> +** Public License - see the file COPYING.LIB in the Mercury distribution.
> +*/
This should be licensed under the GPL rather than the LGPL - it isn't a
library.
> +
> +/*
> +** Author: Ian MacLarty.
> +**
> +** mmc - a wrapper for the Melbourne Mercury Compiler.
> +**
> +** NOTE: Any changes to this file may also require changes to
> +** ../scripts/mmc.in.
> +** This is a C version of ../scripts/mmc.in. This version also
> +** supports the standalone mode (see ../README.Standalone), which
> +** ../scripts/mmc.in does not.
> +** This version of mmc is the one which is installed.
s/which/that/
> +*/
> +
> +#include <stdlib.h>
> +#include <stdio.h>
> +
> +#include "path_util.h"
> +
> +#define MERCURY_COMPILER_RELATIVE_PATH \
> + "lib/mercury/bin/@FULLARCH@/mercury_compile"
> +
> +#define MERCURY_CONFIG_DIR_RELATIVE_PATH \
> + "lib/mercury"
> +
> +#define MERCURY_LINK_CMD_FLAG \
> + "--link-executable-command"
> +
> +#define MERCURY_LINK_EXEC_CMD_FLAG \
> + "--link-executable-command"
> +
> +#define MERCURY_LINK_SHARED_LIB_FLAG \
> + "--link-shared-lib-command"
> +
It would be better if the macros that need to be set by the configure
script were moved to a separate header file, for one thing we may want
to reuse that header file for C versions of the mprof, mdb scripts etc.
> +int
> +main(int argc, char **argv)
> +{
> + char *invoke_mercury_compiler_command;
> + char *env_mercury_compiler;
> + char *env_mercury_config_dir;
> + char *env_mercury_stdlib_dir;
> + char *mercury_compiler_absolute;
> + char *mercury_config_dir_absolute;
> + char *extra_mercury_compiler_flags;
> + char *tmp_ptr;
> + int argnum;
> +
> +#if @MMC_STANDALONE@
> + {
> + char *mercury_bin;
> + char *mercury_home;
> + char *mercury_c_compiler_absolute;
> +
> + mercury_bin = find_exec_dir(argv[0]);
> +
> + if (mercury_bin == NULL) {
> + fprintf(stderr, "mmc: Cannot find path to mmc.\n"
> + "Make sure the directory containing mmc is in your PATH.\n");
> + exit(1);
s/1/EXIT_FAILURE/ here and elsewhere.
> + }
> +
> + tmp_ptr = mercury_bin;
> + mercury_bin = quote_directories_with_spaces(mercury_bin);
> + free(tmp_ptr);
> +
> + set_env_var("PATH", mercury_bin, ENVVAR_PREFIX);
> +
> + mercury_home = new_formatted_string("%s%s..", mercury_bin,
> + DIR_SEP_STR);
> +
> + mercury_compiler_absolute = new_formatted_string("%s%s%s",
> + mercury_home, DIR_SEP_STR, MERCURY_COMPILER_RELATIVE_PATH);
> + convert_dir_separators(mercury_compiler_absolute);
> +
> + mercury_config_dir_absolute = new_formatted_string("%s/%s",
> + mercury_home, MERCURY_CONFIG_DIR_RELATIVE_PATH);
> + convert_dir_separators(mercury_config_dir_absolute);
> + set_env_var("MERCURY_STDLIB_DIR", mercury_config_dir_absolute,
> + ENVVAR_CREATE_IF_UNSET);
> +
> + mercury_c_compiler_absolute = new_formatted_string("%s/%s",
> + mercury_home, "@C_COMPILER_RELATIVE_PATH@");
> + convert_dir_separators(mercury_c_compiler_absolute);
> + set_env_var("MERCURY_C_COMPILER", mercury_c_compiler_absolute,
> + ENVVAR_CREATE_IF_UNSET);
> +
> + extra_mercury_compiler_flags = new_formatted_string(
> + "%s %s/%s %s %s/%s",
> + MERCURY_LINK_EXEC_CMD_FLAG,
> + mercury_home,
> + "@LINK_EXEC_CMD_RELATIVE_PATH@",
> + MERCURY_LINK_SHARED_LIB_FLAG,
> + mercury_home,
> + "@LINK_SHARED_LIB_CMD_RELATIVE_PATH@");
> + convert_dir_separators(extra_mercury_compiler_flags);
> + }
> +#else /* @MMC_STANDALONE@ */
> + mercury_compiler_absolute =
> + copy_string("@LIBDIR@/bin/@FULLARCH@/mercury_compile");
> + mercury_config_dir_absolute =
> + copy_string("@CONFIG_LIBDIR@");
> + extra_mercury_compiler_flags =
> + copy_string("");
> + convert_dir_separators(mercury_compiler_absolute);
> + convert_dir_separators(mercury_config_dir_absolute);
> +#endif /* @MMC_STANDALONE@ */
> +
> + env_mercury_compiler = getenv("MERCURY_COMPILER");
> + env_mercury_config_dir = getenv("MERCURY_CONFIG_DIR");
> + env_mercury_stdlib_dir = getenv("MERCURY_STDLIB_DIR");
> +
> + if (env_mercury_compiler == NULL) {
> + set_env_var("MERCURY_COMPILER", mercury_compiler_absolute,
> + ENVVAR_CREATE_IF_UNSET);
> + } else {
> + mercury_compiler_absolute = env_mercury_compiler;
> + }
I suggest putting a blank line here.
> + if (env_mercury_config_dir == NULL) {
> + if (env_mercury_stdlib_dir == NULL) {
> + set_env_var("MERCURY_CONFIG_DIR", mercury_config_dir_absolute,
> + ENVVAR_CREATE_IF_UNSET);
> + } else {
> + set_env_var("MERCURY_CONFIG_DIR", env_mercury_stdlib_dir,
> + ENVVAR_CREATE_IF_UNSET);
> + }
> + }
> +
> +#if @SET_MACOSX_DEPLOYMENT_TARGET@
> + set_env_var("MACOSX_DEPLOYMENT_TARGET", "10.3", ENVVAR_CREATE_IF_UNSET);
> +#endif
I suggest hash defining:
#define MMC_BASE_MACOSX_DEPLOYMENT_TARGET 10.3
somewhere rather than hardcoding it here.
> +
> + invoke_mercury_compiler_command = new_formatted_string("%s %s",
> + mercury_compiler_absolute, extra_mercury_compiler_flags);
> + for (argnum = 1; argnum < argc; argnum++) {
> + tmp_ptr = invoke_mercury_compiler_command;
> + invoke_mercury_compiler_command = new_formatted_string("%s %s",
> + invoke_mercury_compiler_command, argv[argnum]);
> + free(tmp_ptr);
> + }
> + return system(invoke_mercury_compiler_command);
> +}
> Index: util/path_util.c
> ===================================================================
> RCS file: util/path_util.c
> diff -N util/path_util.c
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ util/path_util.c 1 Oct 2005 08:43:52 -0000
> @@ -0,0 +1,394 @@
> +/*
> +** vim: ts=4 sw=4 expandtab
> +*/
> +
> +/*
> +** Copyright (C) 2005 The University of Melbourne.
> +** This file may only be copied under the terms of the GNU Library General
> +** Public License - see the file COPYING.LIB in the Mercury distribution.
> +*/
> +
> +/*
> +** Author: Ian MacLarty.
> +**
> +** This file implements some utility functions for manipulating paths and
> +** working out were an executable is being executed from.
A little more of an explanation would be useful here.
> +*/
> +
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <stdarg.h>
> +#include <string.h>
> +
> +#include "mercury_conf.h"
> +
> +#include "path_util.h"
> +
> +#ifdef MR_HAVE_UNISTD_H
> +#include <unistd.h>
> +#endif
> +
> +#define MAX_PATH_SIZE 8128
> +
> +static void split_path_into_dir_and_file(char *path, char **dir,
> + char **filename);
> +static int file_exists_and_is_executable(char* path);
> +static char *search_path(char *path, const char *path_separator,
> + char *filename);
> +static char* copy_substring(char* str, int start, int end);
> +static void delete_quotes(char* str);
> +
> +char*
> +find_exec_dir(char* cmd)
> +{
> + char *filename;
> + char *cmd_dir;
> + char *found_dir;
> + char working_dir[MAX_PATH_SIZE];
> + char *absolute_path;
> + char *path_env;
> +
> + split_path_into_dir_and_file(cmd, &cmd_dir, &filename);
> +
> + /*
> + ** Check if the first argument is an absolute path.
> + */
> + if (IS_ABS_PATH(cmd_dir)) {
> + free(filename);
> + return cmd_dir;
> + }
> +
> + /*
> + ** See if the first argument is a path relative to the current working
> + ** directory.
by ...
> + */
> + if (getcwd(working_dir, MAX_PATH_SIZE)) {
> + absolute_path = new_formatted_string("%s%s%s", working_dir,
> + DIR_SEP_STR, cmd);
> + if (file_exists_and_is_executable(absolute_path)) {
> + free(filename);
> + split_path_into_dir_and_file(absolute_path, &found_dir,
> + &filename);
> + free(filename);
> + return found_dir;
> + }
> + }
> +
> + /*
> + ** Search the PATH.
> + */
> + free(cmd_dir);
> + path_env = getenv("PATH");
> +
> + if (path_env != NULL) {
> + found_dir = search_path(path_env, PATH_SEP_STR, filename);
> + free(filename);
> + return found_dir;
> + }
> + free(filename);
> + return NULL;
> +}
> +
> +void
> +convert_dir_separators(char* dir)
> +{
> +#ifdef __WIN32
> + char *chr_ptr;
> +
> + chr_ptr = strchr(dir, '/');
> + while (chr_ptr != NULL) {
> + *chr_ptr = '\\';
> + chr_ptr = strchr(dir, '/');
> + }
> +#endif /* __WIN32 */
> +}
> +
> +void
> +set_env_var(const char *var, char* value, set_env_var_behaviour how)
> +{
> + const char* current_value;
> + char* set_string;
> +
> + current_value = getenv(var);
> + if (current_value == NULL) {
> + current_value = "";
> + } else if (how == ENVVAR_CREATE_IF_UNSET) {
> + return;
> + }
> +
> + switch (how) {
> + case ENVVAR_OVERWRITE:
> + set_string = (char*)malloc(strlen(var) + 1 + strlen(value) + 1);
You'll need to check the return value of malloc here and below. You
should just define a safe version of malloc like we do in the runtime.
> + sprintf(set_string, "%s=%s", var, value);
> + break;
> + case ENVVAR_PREFIX:
> + set_string = (char*)malloc(strlen(var) + 1 + strlen(value)
> + + strlen(PATH_SEP_STR) + strlen(current_value) + 1);
> + sprintf(set_string, "%s=%s%s%s", var, value, PATH_SEP_STR,
> + current_value);
> + break;
> + case ENVVAR_APPEND:
> + set_string = (char*)malloc(strlen(var) + 1 + strlen(current_value)
> + + strlen(PATH_SEP_STR) + strlen(value) + 1);
> + sprintf(set_string, "%s=%s%s%s", var, current_value, PATH_SEP_STR,
> + value);
> + break;
> + case ENVVAR_CREATE_IF_UNSET:
> + set_string = (char*)malloc(strlen(var) + 1 + strlen(value) + 1);
> + sprintf(set_string, "%s=%s", var, value);
> + break;
> + default:
> + fprintf(stderr, "unknown set_env_var_behaviour value");
> + exit(1);
> + }
> +
> + if (putenv(set_string) != 0) {
> + fprintf(stderr, "unable to set environment %s=%s.\n", var, value);
> + exit(1);
> + }
> + /*
> + ** We don't free set_string, since any changes to the contents of
> + ** it will alter the environment (according to the Linux manpage).
> + */
What about non-Linux systems?
> +}
> +
> +char*
> +quote_directories_with_spaces(char *path)
> +{
> + int length;
> + char *new_path;
> + int pos;
> + int found_space;
> + int dir_start;
> + char *one_dir;
> + char *tmp_ptr;
> +
> + length = strlen(path);
> + new_path = copy_string("");
> + found_space = 0;
> + dir_start = 0;
Make the last two into intializers.
> +
> + for (pos = 0; pos < length; pos++) {
> + if (path[pos] == ' ') {
> + found_space = 1;
> + }
> + if (IS_DIR_SEP(path[pos])) {
> + one_dir = copy_substring(path, dir_start, pos - 1);
> + tmp_ptr = new_path;
> + if (found_space) {
> + new_path = new_formatted_string("%s\"%s\"%s", tmp_ptr,
> + one_dir, DIR_SEP_STR);
> + found_space = 0;
> + } else {
> + new_path = new_formatted_string("%s%s%s", tmp_ptr,
> + one_dir, DIR_SEP_STR);
> + }
> + free(tmp_ptr);
> + free(one_dir);
> + dir_start = pos + 1;
> + }
> + }
> + one_dir = copy_substring(path, dir_start, pos - 1);
> + tmp_ptr = new_path;
> + if (found_space) {
> + new_path = new_formatted_string("%s\"%s\"", tmp_ptr, one_dir);
> + found_space = 0;
> + } else {
> + new_path = new_formatted_string("%s%s", tmp_ptr, one_dir);
> + }
> + free(tmp_ptr);
> + free(one_dir);
> +
> + return new_path;
> +}
> +
> +/*
> +** Split the given path into directory and filename components.
> +** *dir and *filename are set to newly alloctated strings containing the
> +** directory and filename parts of the path respectively.
> +*/
> +
> +static void
> +split_path_into_dir_and_file(char *path, char **dir, char **filename)
> +{
> + char *last_dir_sep;
> +
> + last_dir_sep = strrchr(path, DIR_SEP_CHR);
> + if (last_dir_sep != NULL) {
> + *filename = copy_string((char*)(last_dir_sep + 1));
> + *dir = copy_substring(path, 0, last_dir_sep - path - 1);
> + } else {
> + *filename = copy_string(path);
> + *dir = copy_string("");
> + }
> +}
> +
> +/*
> +** Return 1 if the given file exists and is executable. Otherwise
> +** return 0. On Windows this function will append ".exe" to the filename
> +** before checking if it exists if there is no ".exe" there already
> +** (the string pointed to by path remains unchanged).
> +*/
> +
> +static int
> +file_exists_and_is_executable(char* path)
> +{
> + FILE *fp;
> + char *exe_pos;
> + char path_with_exe[MAX_PATH_SIZE];
> +
> + if (strlen(path) >= MAX_PATH_SIZE) {
> + return 0;
> + }
> +#if defined(__WIN32) || defined(__CYGWIN__)
I suggest letting configure work out any system specific
details and just defining MMC_WIN32, MMC_CYGWIN etc - perhaps
you could just reuse the ones in mercury_conf.h?
> + exe_pos = strstr(path, ".exe");
> + if (exe_pos == NULL || strcmp(exe_pos, ".exe") != 0) {
> + if (snprintf(path_with_exe, MAX_PATH_SIZE - 1, "%s%s", path, ".exe")
> + >= MAX_PATH_SIZE)
> + {
> + return 0;
> + }
> + } else {
> + strncpy(path_with_exe, path, MAX_PATH_SIZE - 1);
> + }
> +#else /* defined(__WIN32) || defined(__CYGWIN__) */
> + strncpy(path_with_exe, path, MAX_PATH_SIZE - 1);
> +#endif /* defined(__WIN32) || defined(__CYGWIN__) */
> +
> +#ifdef MR_HAVE_UNISTD_H
> + if (access(path_with_exe, X_OK) == 0) {
> + return 1;
> + }
> +#else
> + fp = fopen(path_with_exe, "r");
> + if (fp != NULL) {
> + fclose(fp);
> + return 1;
> + }
> +#endif /* MR_HAVE_UNISTD_H */
> +
> + return 0;
> +}
> +
> +/*
> +** Search the given path for the executable file given by filename,
> +** using path_separator as the path separator (`:' on unix, `;' on Windows).
> +** If the file is found then return the path to the directory containing the
> +** file, otherwise return NULL.
> +** The returned string will be in a newly allocated block of memory.
> +*/
> +
> +static char*
> +search_path(char *path, const char *path_separator, char *filename)
> +{
> + char *dir;
> + char *dirs;
> + char *found_dir;
> + char *absolute_path;
> +
> + dirs = copy_string(path);
> +#ifdef _WIN32
> + /*
> + ** On Windows, directory names that need to be quoted (for example
> + ** directory names with spaces) may have the quote characters in the PATH
> + ** entry.
> + */
> + delete_quotes(dirs);
> +#endif
> +
> + dir = (char*)strtok(dirs, path_separator);
The cast to char * is redundant here and below.
> + do {
> + absolute_path = new_formatted_string("%s/%s", dir, filename);
> + convert_dir_separators(absolute_path);
> + if (file_exists_and_is_executable(absolute_path)) {
> + found_dir = copy_string(dir);
> + free(dirs);
> + return found_dir;
> + }
> + dir = (char*)strtok(NULL, path_separator);
> + } while (dir != NULL);
> +
> + free(dirs);
> + return NULL;
> +}
> +
> +/*
> +** The following code is taken from the sprintf Linux man page
That explains a bit ;-)
> +** (slightly modified).
> +*/
> +char *
> +new_formatted_string(const char *fmt, ...) {
> + /* Guess we need no more than 100 bytes. */
> + int n;
> + int size = 100;
> + char *p;
> + va_list ap;
These variable names are not very good. Also the type
of size should be `size_t'.
> +
> + if ((p = malloc (size)) == NULL) {
> + return NULL;
> + }
> + while (1) {
> + /* Try to print in the allocated space. */
> + va_start(ap, fmt);
> + n = vsnprintf (p, size, fmt, ap);
> + va_end(ap);
> + /* If that worked, return the string. */
> + if (n > -1 && n < size) {
> + return p;
> + }
> + /* Else try again with more space. */
> + if (n > -1) { /* glibc 2.1 */
> + size = n+1; /* precisely what is needed */
> + } else { /* glibc 2.0 */
> + size *= 2; /* twice the old size */
> + }
> + if ((p = realloc (p, size)) == NULL) {
> + return NULL;
> + }
> + }
> +}
> +
> +char*
> +copy_string(const char* str)
> +{
> + char* new_str;
> +
> + new_str = malloc(strlen(str) + 1);
> + strcpy(new_str, str);
> + return new_str;
> +}
> +
> +static char*
> +copy_substring(char* str, int start, int end)
> +{
> + char* new_str;
> +
> + if (end < start) {
> + new_str = copy_string("");
> + return new_str;
> + } else {
> + new_str = malloc(end - start + 2);
> + strncpy(new_str, &str[start], end - start + 1);
> + new_str[end - start + 1] = '\0';
> + return new_str;
> + }
> +}
> +
> +static void
> +delete_quotes(char* str)
> +{
> + int read_pos;
> + int write_pos;
> + int len;
> +
> + len = strlen(str);
> + write_pos = 0;
> + for (read_pos = 0; read_pos < len; read_pos++) {
> + if (str[read_pos] != '\"') {
> + str[write_pos] = str[read_pos];
> + write_pos++;
> + }
> + }
> + str[write_pos] = '\0';
> +}
> Index: util/path_util.h
> ===================================================================
> RCS file: util/path_util.h
> diff -N util/path_util.h
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ util/path_util.h 1 Oct 2005 08:46:19 -0000
> @@ -0,0 +1,100 @@
> +/*
> +** vim: ts=4 sw=4 expandtab
> +*/
> +/*
> +** Copyright (C) 2005 The University of Melbourne.
> +** This file may only be copied under the terms of the GNU Library General
> +** Public License - see the file COPYING.LIB in the Mercury distribution.
> +*/
> +
> +/*
> +** Author: Ian MacLarty.
> +**
> +** This file exports some utility functions for manipulating paths and
> +** working out were an executable is being executed from.
> +*/
> +
> +/*
> +** Find the location of the executable that was invoked with the
> +** command cmd.
> +** This function first checks if cmd contains an absolute path,
> +** then if it contains a path relative to the current working directory,
> +** and finally it searches the PATH for an executable file with the same
> +** name as the file given in cmd.
> +** On success a newly allocated string containing the directory is returned,
> +** otherwise NULL is returned.
> +*/
> +
> +extern char* find_exec_dir(char* cmd);
> +
That should probably be `const char *cmd'. It looks like there are
quite a few char pointer arguments arounds that could be const
qualified (actually I would be suprised if it compiles without getting
a few warnings about discarded qualifiers as is).
> +/*
> +** This function converts any `/' characters in the given string to `\'
> +** characters if __WIN32 is defined. If __WIN32 is undefined then
> +** the string is left unchanged.
> +*/
> +
> +extern void convert_dir_separators(char* dir);
> +
> +/*
> +** Change the given environment variable.
> +** The how argument says how the environment variable should be changed.
That sentence is quite awkward, at the very least it should be:
The 'how' argument says how ...
> +** For ENVVAR_PREFIX and ENVVAR_APPEND, the default path seperator will be
Guess what the problem here is ;-)
> +** used to seperate the old value from the prefixed or appended value
and here.
> +** (on unix the path seperator is `:', while on Windows it is `;').
> +** If memory could not be allocated for the environment variable then
If memory cannot be allocated for a copy of the environment variable...?
> +** this function displays an error and aborts the program.
> +*/
> +
> +typedef enum {
> + ENVVAR_OVERWRITE,
> + ENVVAR_PREFIX,
> + ENVVAR_APPEND,
> + ENVVAR_CREATE_IF_UNSET
> +} set_env_var_behaviour;
> +
> +extern void set_env_var(const char *var, char* value, set_env_var_behaviour how);
> +
> +/*
> +** Put quotes around directories that contain spaces in the given path
> +** and return a newly allocated string containing the result.
> +*/
I suggest:
Return newly allocated string containing the given path with quotes placed
around any directory names that contain spaces.
> +
> +extern char* quote_directories_with_spaces(char *path);
> +
> +/*
> +** new_formatted_string creates a new string based on the format string fmt
> +** and the rest of its arguments and returns the result in a newly allocated
Replace the and with a comma.
> +** string.
> +*/
> +
> +extern char *new_formatted_string(const char *fmt, ...);
> +
> +/*
> +** Return a reference to a copy of the given string.
> +*/
> +
> +extern char* copy_string(const char* str);
> +
> +#ifdef _WIN32
> +
> +#define IS_DIR_SEP(c) (c == '\\')
> +#define DIR_SEP_STR "\\"
> +#define DIR_SEP_CHR '\\'
> +#define PATH_SEP_STR ";"
> +#define WIN32_DRIVE_LETTERS \
> + "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"
> +#define IS_ABS_PATH(p) \
> + ((strlen(p) >= 3 && strchr(WIN32_DRIVE_LETTERS, p[0]) && \
> + p[1] == ':' && p[2] == '\\') \
> + || (strlen(p) >= 2 && p[0] == '\\' && p[1] == '\\'))
> +
> +#else /* ifdef _WIN32 */
> +
> +#define IS_DIR_SEP(c) (c == '/')
> +#define DIR_SEP_STR "/"
> +#define DIR_SEP_CHR '/'
> +#define PATH_SEP_STR ":"
> +#define IS_ABS_PATH(p) (p[0] == '/')
> +
> +#endif /* ifdef _WIN32 */
> +
That's all for now.
Cheers,
Julien.
--------------------------------------------------------------------------
mercury-reviews mailing list
post: mercury-reviews at cs.mu.oz.au
administrative address: owner-mercury-reviews at cs.mu.oz.au
unsubscribe: Address: mercury-reviews-request at cs.mu.oz.au Message: unsubscribe
subscribe: Address: mercury-reviews-request at cs.mu.oz.au Message: subscribe
--------------------------------------------------------------------------
More information about the reviews
mailing list