[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