[m-dev.] For review: using alternate installation dirs, round 2
Fergus Henderson
fjh at cs.mu.OZ.AU
Sat Jul 24 01:23:54 AEST 1999
On 23-Jul-1999, Warwick Harvey <wharvey at cs.monash.edu.au> wrote:
> RCS file: /home/mercury1/repository/mercury/extras/references/samples/Mmakefile,v
> retrieving revision 1.2
> diff -u -r1.2 Mmakefile
> --- Mmakefile 1999/07/20 03:39:23 1.2
> +++ Mmakefile 1999/07/23 02:46:56
> @@ -9,14 +9,8 @@
> # We need to use a grade with trailing
> GRADEFLAGS += --use-trail
>
> -MGNUCFLAGS= -I..
> -
> -# Link in the reference library from ..
> -MCFLAGS += -I.. $(EXTRA_MCFLAGS)
> -MLFLAGS += -R`pwd`/.. -L.. $(EXTRA_MLFLAGS)
> -MLLIBS = -lglobal $(EXTRA_MLLIBS)
> -VPATH = ..:$(MMAKE_VPATH)
> -C2INITARGS = ../global.init
> +EXTRA_LIB_DIRS = /home/wharvey/mercury/libdirs/extras/lib/mercury
> +EXTRA_LIBRARIES = global
I know this is just a sample diff, not one that you're going to commit,
but shouldn't that be
EXTRA_LIB_DIRS = ..
rather than using a hard-coded path name? I think now that ml
expands relative path names there should be no need to use an
absolute path here.
> Index: scripts/mmake.in
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/scripts/mmake.in,v
> retrieving revision 1.28
> diff -u -r1.28 mmake.in
> --- mmake.in 1999/07/20 03:39:18 1.28
> +++ mmake.in 1999/07/23 02:46:57
> @@ -66,6 +66,8 @@
> MMAKE_VARS=${MMAKE_VARS=$MMAKE_DIR/Mmake.vars}
> MMAKE_RULES=${MMAKE_RULES=$MMAKE_DIR/Mmake.rules}
> MERCURY_INT_DIR=${MERCURY_INT_DIR=@LIBDIR@/ints}
> +MERCURY_EXTRA_INT_DIRS=${MERCURY_EXTRA_INT_DIRS=}
> +MERCURY_EXTRA_INIT_LIB_DIRS=${MERCURY_EXTRA_INIT_LIB_DIRS=}
> MERCURY_DEFAULT_GRADE=${MERCURY_DEFAULT_GRADE=@DEFAULT_GRADE@}
> MKTEMP=@MKTEMP@
Adding these variable definitions here doesn't make sense, because
those definitions are not used. Perhaps you intended to also
export them?
> Index: util/mkinit.c
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/util/mkinit.c,v
> retrieving revision 1.52
> diff -u -r1.52 mkinit.c
> --- mkinit.c 1999/06/29 19:08:32 1.52
> +++ mkinit.c 1999/07/23 02:46:57
> @@ -23,6 +23,8 @@
> #include <string.h>
> #include <ctype.h>
> #include <errno.h>
> +#include <unistd.h>
> +#include <sys/stat.h>
Those header files are part of POSIX, but they are not standard ANSI/ISO C,
so you should avoid using them if possible.
If not, then you really ought to use autoconf to check that they exist,
and protect the #include statements with #ifdefs, and do something
appropriate in the case when they don't exist.
In this particular case, where you were just using stat() to check the
existence of a file, the appropriate thing to do if they don't
exist is to use fopen() instead of stat(). (Of course, don't forget
to fclose() the file afterwards.)
There's an efficiency/portability/simplicity trade-off here
(choose any two). The efficient and portable solution using #ifdef
would be best, IMHO, but is the most work. I'd be satisfied with the
simple and portable but not maximally efficient version using fopen().
But I would rather not have the simple and efficient but not maximally
portable solution using stat().
Anyway, I think at very least it's worth abstracting the code to check
whether a file exists into a separate function.
static bool file_exists(const char *filename);
...
> + case 'I':
> + tmp_slist = (String_List *)
> + checked_malloc(sizeof(String_List));
> + tmp_slist->next = NULL;
> + tmp_slist->data = (char *)
> + checked_malloc(strlen(optarg) + 1);
> + strcpy(tmp_slist->data, optarg);
> + *init_file_dirs_tail = tmp_slist;
> + init_file_dirs_tail = &tmp_slist->next;
> + break;
> +
It would be good to add a comment here saying "append the directory
name into the end of the list" or something like that.
--
Fergus Henderson <fjh at cs.mu.oz.au> | "I have always known that the pursuit
WWW: <http://www.cs.mu.oz.au/~fjh> | of excellence is a lethal habit"
PGP: finger fjh at 128.250.37.3 | -- the last words of T. S. Garp.
--------------------------------------------------------------------------
mercury-developers mailing list
Post messages to: mercury-developers at cs.mu.oz.au
Administrative Queries: owner-mercury-developers at cs.mu.oz.au
Subscriptions: mercury-developers-request at cs.mu.oz.au
--------------------------------------------------------------------------
More information about the developers
mailing list