[m-rev.] for review: implement C version of mmc
Ian MacLarty
maclarty at cs.mu.OZ.AU
Fri Oct 21 09:16:08 AEST 2005
On Thu, 20 Oct 2005, Julien Fischer wrote:
>
> On Thu, 20 Oct 2005, Ian MacLarty wrote:
>
> > > > 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?
> > >
> >
> > There is no relationship.
> >
> We discussed this in person this morning.
>
I have changed the description of --with-relative-cc in README.Standalone to
the following:
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).
The given C compiler will be used by the built Mercury compiler to
compile the C code it generates. It will not be used to build the
compiler itself. To specify the C compiler to use to build the
compiler itself, use the --with-cc option.
> > > > ===================================================================
> > > > 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?
> > >
> >
> > -programs. The directory containing the Mercury and C compilers can be moved
> > +programs (Mercury will still require the `sort' program to compile
> > +programs that make use of :- pragma fact_table).
> > +
> >
> The appropriate README file (Standalone or Windows?) should mention that
> fact table's won't (yet) work then.
>
They should work if the user has a sort program installed, which is what the
above comment says.
> > diff -u util/path_util.c util/path_util.c
> > --- util/path_util.c 1 Oct 2005 08:43:52 -0000
> > +++ util/path_util.c 19 Oct 2005 13:31:19 -0000
> > @@ -12,7 +12,12 @@
> > ** Author: Ian MacLarty.
> > **
> > ** This file implements some utility functions for manipulating paths and
> > -** working out were an executable is being executed from.
> > +** working out were an executable is being run from.
> > +** The functions in this file are used by mmc.c when it is in standalone
> > +** mode to work out where it is being run from, so that it can pass the
> > +** absolute paths to the configuration and library directories and the
> > +** absolute paths of the C compiler and linker to mercury_compile.
> > +** It also contains a few string manipulation functions used by mmc.
> > */
> >
> >
> > @@ -22,6 +27,7 @@
> > #include <string.h>
> >
> > #include "mercury_conf.h"
> > +#include "mercury_conf_param.h"
> >
> > #include "path_util.h"
> >
> > @@ -31,16 +37,16 @@
> >
> > #define MAX_PATH_SIZE 8128
> >
> > -static void split_path_into_dir_and_file(char *path, char **dir,
> > +static void split_path_into_dir_and_file(const 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 int file_exists_and_is_executable(const char *path);
> > +static char *search_path(const char *path, const char *path_separator,
> > + const char *filename);
> > +static char* copy_substring(const char *str, int start, int end);
> > static void delete_quotes(char* str);
> >
> > char*
> > -find_exec_dir(char* cmd)
> > +find_exec_dir(const char *cmd)
> > {
> > char *filename;
> > char *cmd_dir;
> > free(filename);
>
> ...
>
> > -copy_substring(char* str, int start, int end)
> > +copy_substring(const char *str, int start, int end)
> > {
> > char* new_str;
> >
> > @@ -371,6 +415,11 @@
> > new_str = malloc(end - start + 2);
> > strncpy(new_str, &str[start], end - start + 1);
> > new_str[end - start + 1] = '\0';
> > + if (new_str == NULL) {
> > + fprintf(stderr, "Error: Could not allocate %i bytes of memory\n",
> > + strlen(str) + 1);
> > + exit(EXIT_FAILURE);
> > + }
>
> That looks wrong. The check for malloc returning NULL should occur before
> you attempt to use the memory it allocates.
>
Yes I got that the wrong way round - thanks.
> ...
>
> > +#ifdef MR_HAVE_UNISTD_H
> > + /*
> > + ** We have execv, so set up the argument vector and call execv.
> > + */
> > + {
> > + char **mercury_compiler_args;
> > + int num_args;
> > + int current_arg;
> > + int current_orig_arg;
> > +
> > + num_args = 0;
> > + if (strcmp(link_exec_cmd_absolute_path, "") != 0) {
> > + num_args += 2;
> > + }
> > + if (strcmp(link_shared_lib_cmd_absolute_path, "") != 0) {
> > + num_args += 2;
> > + }
> > + num_args += argc;
> > + mercury_compiler_args = (char **)
> > + calloc(num_args + 1, sizeof(char **));
>
> You need to make sure that calloc doesn't return NULL here.
>
> Once this has boostrapped you should modify tools/test_mercury so
> that the nightly tests on one of the machines (not the rotd host for
> now) use the C version of mmc.
>
Okay.
Thanks for the review.
Ian.
--------------------------------------------------------------------------
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