From bug-octave-request at bevo dot che dot wisc dot edu Fri Oct 23 02:39:39 1998 Subject: [patch] extra colon feature + problem with default loadpath From: Rafael Laboissiere To: bug-octave at bevo dot che dot wisc dot edu Cc: Dirk Eddelbuettel Date: Fri, 23 Oct 1998 09:40:42 +0200 (METDST) --Multipart_Fri_Oct_23_09:40:42_1998-1 Content-Type: text/plain; charset=US-ASCII -------- Bug report for Octave 2.0.13 configured for i386-pc-linux-gnu Description: ----------- Some days ago I sent a message to help-octave (http://www.che.wisc.edu/octave/mailing-lists/help-octave/1998/780), asking for a implementation of the doubled colon feature of kpathsea. Johan Eaton proposed a patch to defaults.cc, but I found a way to solve the problem using the available functions in libkpathsea. [kpse_expand_default(), see my suggestion in http://www.che.wisc.edu/octave/mailing-lists/help-octave/1998/787]. The solution found, implied changing a couple of files in version 2.0.13. (BTW, guys, reading the Octave source is a great pleasure, and making modifications to it are quite straightforward; Congrats!) Essentially, what I did was to transfer the burden of default path expansion from file src/defaults.cc to the method dir_path::init() [in file liboctave/pathsearch.cc]. A new global variable Vload_path_default is defined , that stores the default path. A new builtin function Fload_path_default() was also created, which prints the default path (this addition is fully optional, but is the only way that I could imagine to get access to the value of the default path from the Octave prompt). The default value for Vload_path (and hence LOADPATH at startup) is set to ":". This adds a nice feature to Octave, namely, that initializations of LOADPATH in octave.conf or ~/.octaverc can safely append/prepend values to LOADPATH, and the default path is still there. As a counterexemple, the file /etc/octave.conf in a Debian system (Salut Dirk !) has the line: LOADPATH = [ ":/usr/local/share/octave/site-m//", LOADPATH ]; With the current version of Octave, after initialization we get the following value for LOADPATH: :/usr/local/share/octave/site-m//.:/usr/lib/octave/site/oct/i386-pc-linux-gnu//:/usr/share/octave/site/m//:/usr/lib/octave/2.0.13/oct/i386-pc-linux-gnu//:/usr/share/octave/2.0.13/m// whose second element is wrong (more on that below). The function maybe_add_default_load_path() is removed, because its job is done by kpse_expand_default(). IMHO, this increases consistency and has the benefit of reducing redundancy and of cleaning up the code. In the process of implementing the doubled colon feature, I fixed a bug and a bad feature. First, the bug: the environment variable OCTAVE_PATH, if it exists, is intended to replace the default path (defined by #define OCTAVE_FCNFILEPATH). However, as the function maybe_add_default_load_path() uses the value OCTAVE_FCNFILEPATH, the environment variable OCTAVE_PATH is useless. My design solves the problem and allow the user to _really_ overriden the builtin default path. The bad feature is illustrated above: at initialization, if LOADPATH has leading or trailing colons, the default path is inserted in it. With my new design, no insertion happens, the default path is kept hidden, thanks to Vload_path_default. This means that the user gets the value of LOADPATH that she asked for. Repeat-By: --------- To replicate the bug, just do (version 2.0.13): $ export OCTAVE_PATH=foobar $ octave --no-site-file --no-init-file Octave, version 2.0.13 (i386-pc-linux-gnu). Copyright (C) 1996, 1997, 1998 John W. Eaton. This is free software with ABSOLUTELY NO WARRANTY. For details, type `warranty'. octave:1> LOADPATH LOADPATH = foobar octave:2> which freqz which: `freqz' is undefined octave:3> LOADPATH = [ ":barfuz:", LOADPATH ] LOADPATH = :barfuz:foobar octave:4> which freqz freqz is the function defined from: /usr/share/octave/2.0.13/m/signal/freqz.m Constrast with the output of my modified version: $ export OCTAVE_PATH=foobar $ octave --no-site-file --no-init-file LOADPATH (octave_loadpath): : Vload_path_dir_path.p (octave_loadpath): Octave, version 2.0.13 (i386-pc-linux-gnu). Copyright (C) 1996, 1997, 1998 John W. Eaton. This is free software with ABSOLUTELY NO WARRANTY. For details, type `warranty'. octave:1> LOADPATH LOADPATH = : octave:2> which freqz which: `freqz' is undefined octave:3> LOADPATH = [ LOADPATH, "barfuz" ] LOADPATH = :barfuz octave:4> which freqz which: `freqz' is undefined Fix: --- I am attaching the patch at the end of this message. I tried to follow as close as possible the style of the Octave source. I did not put many comments on the source itself, but my Changelog entries (both under src/ and liboctave/) are quite comprehensive. Besides, the present bug report should add clarity. There is a problem in a call to free() that I could not solve (in method dir_path::init(), file liboctave/pathsearch.cc). My feeling is that the tmp variable should be freed, but by doing that octave dumps core. Any free/malloc wizard? I apologize for sending a so big patch with many modifications, but the issues that I addressed had to be solved together. Last comment: I did not check the latest development release to see if the issues here were already addressed. I am interested in getting Octave fixed for the Debian distribution (which includes the latest stable version 2.0.13). [Pour Dirk : j'ai essaye ce patch avec la version 2.0.13-6 de ton package octave.]. Configuration (please do not edit this section): ----------------------------------------------- uname output: Linux debian 2.0.32 #1 Wed Jan 28 10:26:12 CET 1998 i586 unknown configure opts: --prefix=/usr --libdir=/usr/lib --libexecdir=/usr/lib --with-g77 --enable-dl --enable-shared --enable-lite-kernel --host i386-linux Fortran compiler: g77 FFLAGS: -O2 F2C: F2CFLAGS: FLIBS: -lg2c -lm -L/usr/lib/gcc-lib/i486-linux/egcs-2.91.56 -lm CPPFLAGS: INCFLAGS: -I/usr/include -I/usr/include/octave-2.0.13 C compiler: egcc, version 2.7.2.3 CFLAGS: -DHAVE_CONFIG_H -mieee-fp -O2 CPICFLAG: -fPIC C++ compiler: c++, version 2.91.57 19980901 (egcs-1.1 release) CXXFLAGS: -DHAVE_CONFIG_H -mieee-fp -fno-rtti -fno-exceptions -fno-implicit-templates -O2 CXXPICFLAG: -fPIC LDFLAGS: -s LIBFLAGS: -L/usr/lib/octave-2.0.13 RLD_FLAG: -Xlinker -rpath -Xlinker /usr/lib/octave-2.0.13 TERMLIBS: -lncurses LIBS: LEXLIB: LIBPLPLOT: LIBDLFCN: DEFS: -DOCTAVE_SOURCE=1 -DSEPCHAR=':' -DSEPCHAR_STR=":" -DUSE_READLINE=1 -DCXX_NEW_FRIEND_TEMPLATE_DECL=1 -DF77_APPEND_UNDERSCORE=1 -DOCTAVE_LITE=1 -DSIZEOF_SHORT=2 -DSIZEOF_INT=4 -DSIZEOF_LONG=4 -DHAVE_ALLOCA_H=1 -DHAVE_ALLOCA=1 -DNPOS=string::npos -DSTDC_HEADERS=1 -DHAVE_DIRENT_H=1 -DTIME_WITH_SYS_TIME=1 -DHAVE_SYS_WAIT_H=1 -DHAVE_ASSERT_H=1 -DHAVE_CURSES_H=1 -DHAVE_DLFCN_H=1 -DHAVE_FCNTL_H=1 -DHAVE_FLOAT_H=1 -DHAVE_FNMATCH_H=1 -DHAVE_GLOB_H=1 -DHAVE_GRP_H=1 -DHAVE_LIMITS_H=1 -DHAVE_MEMORY_H=1 -DHAVE_NAN_H=1 -DHAVE_NCURSES_H=1 -DHAVE_PWD_H=1 -DHAVE_SGTTY_H=1 -DHAVE_STDLIB_H=1 -DHAVE_STRING_H=1 -DHAVE_SYS_PARAM_H=1 -DHAVE_SYS_RESOURCE_H=1 -DHAVE_SYS_SELECT_H=1 -DHAVE_SYS_STAT_H=1 -DHAVE_SYS_TIME_H=1 -DHAVE_SYS_TIMES_H=1 -DHAVE_SYS_TYPES_H=1 -DHAVE_SYS_UTSNAME_H=1 -DHAVE_TERMCAP_H=1 -DHAVE_TERMIO_H=1 -DHAVE_TERMIOS_H=1 -DHAVE_UNISTD_H=1 -DHAVE_VARARGS_H=1 -DHAVE_FNMATCH=1 -DHAVE_GLOB=1 -DHAVE_ATEXIT=1 -DHAVE_BCOPY=1 -DHAVE_BZERO=1 -DHAVE_DUP2=1 -DHAVE_ENDGRENT=1 -DHAVE_ENDPWENT=1 -DHAVE_EXECVP=1 -DHAVE_FCNTL=1 -DHAVE_FORK=1 -DHAVE_GETCWD=1 -DHAVE_GETEGID=1 -DHAVE_GETEUID=1 -DHAVE_GETGID=1 -DHAVE_GETGRENT=1 -DHAVE_GETGRGID=1 -DHAVE_GETGRNAM=1 -DHAVE_GETHOSTNAME=1 -DHAVE_GETPGRP=1 -DHAVE_GETPID=1 -DHAVE_GETPPID=1 -DHAVE_GETPWENT=1 -DHAVE_GETPWNAM=1 -DHAVE_GETPWUID=1 -DHAVE_GETTIMEOFDAY=1 -DHAVE_GETUID=1 -DHAVE_GETWD=1 -DHAVE_LSTAT=1 -DHAVE_MEMMOVE=1 -DHAVE_MKDIR=1 -DHAVE_MKFIFO=1 -DHAVE_ON_EXIT=1 -DHAVE_PIPE=1 -DHAVE_PUTENV=1 -DHAVE_RENAME=1 -DHAVE_RINDEX=1 -DHAVE_RMDIR=1 -DHAVE_SETGRENT=1 -DHAVE_SETPWENT=1 -DHAVE_SETVBUF=1 -DHAVE_SIGACTION=1 -DHAVE_SIGPENDING=1 -DHAVE_SIGPROCMASK=1 -DHAVE_SIGSUSPEND=1 -DHAVE_STAT=1 -DHAVE_STRCASECMP=1 -DHAVE_STRDUP=1 -DHAVE_STRERROR=1 -DHAVE_STRNCASECMP=1 -DHAVE_TEMPNAM=1 -DHAVE_UMASK=1 -DHAVE_UNLINK=1 -DHAVE_USLEEP=1 -DHAVE_VFPRINTF=1 -DHAVE_VSPRINTF=1 -DHAVE_WAITPID=1 -DSMART_PUTENV=1 -DHAVE_PROGRAM_INVOCATION_NAME=1 -DHAVE_LIBDL=1 -DHAVE_DLOPEN=1 -DHAVE_DLSYM=1 -DHAVE_DLERROR=1 -DHAVE_DLCLOSE=1 -DWITH_DL=1 -DWITH_DYNAMIC_LINKING=1 -DHAVE_LIBM=1 -DHAVE_FINITE=1 -DHAVE_ISNAN=1 -DHAVE_ISINF=1 -DHAVE_ACOSH=1 -DHAVE_ASINH=1 -DHAVE_ATANH=1 -DHAVE_ERF=1 -DHAVE_ERFC=1 -DHAVE_ST_BLKSIZE=1 -DHAVE_ST_BLOCKS=1 -DHAVE_ST_RDEV=1 -DHAVE_TM_ZONE=1 -DHAVE_GR_PASSWD=1 -DEXCEPTION_IN_MATH=1 -DRETSIGTYPE=void -DSYS_SIGLIST_DECLARED=1 -DHAVE_SYS_SIGLIST=1 -DHAVE_POSIX_SIGNALS=1 -DHAVE_GETRUSAGE=1 -DHAVE_TIMES=1 -DGNUPLOT_HAS_MULTIPLOT=1 -DGNUPLOT_HAS_FRAMES=1 --Multipart_Fri_Oct_23_09:40:42_1998-1 Content-Type: application/octet-stream; type=patch Content-Disposition: attachment; filename="octave-2.0.13-kpse-expand-default.patch" Content-Transfer-Encoding: binary --- octave-2.0.13.orig/liboctave/ChangeLog +++ octave-2.0.13/liboctave/ChangeLog at @ -1,3 +1,17 @@ +Thu Oct 22 18:15:01 1998 Rafael Laboissiere + + * pathsearch.h (Vload_path_default): Added declaration. + This variable is initialized in src/defaults.cc. + * pathsearch.cc (Vload_path_default): Added definition. + (dir_path::init): Changed the private method init() to call + kpse_expand_default(), defined in kpathsea/kdefault.c. Every time + that a dir_path variable is created, we try to expand "extra + colons" (i.e., leading, trailing, or doubled) to the default path. + This feature of Kpathsea was not yet fully implemented in previous + versions. + (#includes): Included kpathsea/default.h. It is necessary in + order to access kpse_expand_default(). + Thu May 21 13:15:36 1998 John W. Eaton * Version 2.0.13 released. --- octave-2.0.13.orig/liboctave/pathsearch.cc +++ octave-2.0.13/liboctave/pathsearch.cc at @ -40,12 +40,15 @@ #include #include #include +#include extern unsigned int kpathsea_debug; #undef string } +string Vload_path_default; + static bool octave_kpathsea_initialized = false; string dir_path::program_name; at @ -156,6 +159,19 @@ octave_kpathsea_initialized = true; } + + if (! Vload_path_default.empty ()) { + char *tmp = kpse_expand_default (p_orig.c_str (), + Vload_path_default.c_str ()); + if (tmp) + { + p_orig = tmp; + // FIXME !!! Do not know why freeing tmp here produces a core dump + // free(tmp); + } + else + p_orig = string (); + } char *tmp = kpse_path_expand (p_orig.c_str ()); if (tmp) --- octave-2.0.13.orig/liboctave/pathsearch.h +++ octave-2.0.13/liboctave/pathsearch.h at @ -27,6 +27,11 @@ #include "str-vec.h" +// Default load path. Vload_path gets its value by default at startup. +// It is also used for default path expansion, following kpathsea +// convention (leading, trailing, or doubled colon in path) +extern string Vload_path_default; + class dir_path { at @ -89,6 +94,7 @@ static string program_name; void init (void); + }; #endif --- octave-2.0.13.orig/src/ChangeLog +++ octave-2.0.13/src/ChangeLog at @ -1,3 +1,25 @@ +Thu Oct 22 18:15:01 1998 Rafael Laboissiere + + * defaults.cc (octave_loadpath): Do default path expansion using a + function in the KPathSea library. This is done when variable + Vload_path_dir_path is initialized. + (octave_loadpath): Variable Vload_path_default (defined in + liboctave/pathsearch.cc) is initialized. + (octave_loadpath): The initial value for Vload_path (and hence + LOADPATH) will be ":", which force expansion to the default load + path. This is also nice for path concatenenation to LOADPATH in + user initialization scripts. + (maybe_add_default_load_path): This function was removed, as it is + no more needed. + * defaults.h.in (maybe_add_default_load_path): Removed the prototype + for this function. + * utils.cc (DEFUN (file_in_path)): There is no need anymore for + calling maybe_add_default_load_path(), as the default path + expansion is done in dir_path::init() (see + search_path_for_file() for the creation of the dir_path variable). + (DEFUN(load_path_default)): Created this function for obtaining + the default load path from the Octave prompt. + Thu May 21 13:15:36 1998 John W. Eaton * Version 2.0.13 released. --- octave-2.0.13.orig/src/defaults.h.in +++ octave-2.0.13/src/defaults.h.in at @ -146,8 +146,6 @@ extern string Vlocal_site_defaults_file; extern string Vsite_defaults_file; -extern string maybe_add_default_load_path (const string& pathstring); - extern void install_defaults (void); extern void symbols_of_defaults (void); --- octave-2.0.13.orig/src/defaults.cc +++ octave-2.0.13/src/defaults.cc at @ -191,7 +191,9 @@ char *oct_path = getenv ("OCTAVE_PATH"); - Vload_path = oct_path ? string (oct_path) : std_path; + Vload_path_default = oct_path ? string (oct_path) : std_path; + + Vload_path = string (":"); Vload_path_dir_path = dir_path (Vload_path); } at @ -242,30 +244,6 @@ Vsite_defaults_file.append ("/octaverc"); } -string -maybe_add_default_load_path (const string& pathstring) -{ - string std_path = subst_octave_home (OCTAVE_FCNFILEPATH); - - string retval; - - if (! pathstring.empty ()) - { - if (pathstring[0] == SEPCHAR) - { - retval = std_path; - retval.append (pathstring); - } - else - retval = pathstring; - - if (pathstring[pathstring.length () - 1] == SEPCHAR) - retval.append (std_path); - } - - return retval; -} - void install_defaults (void) { at @ -429,7 +407,7 @@ } else { - Vload_path = maybe_add_default_load_path (s); + Vload_path = s; Vload_path_dir_path = dir_path (Vload_path); } --- octave-2.0.13.orig/src/utils.cc +++ octave-2.0.13/src/utils.cc at @ -263,9 +263,7 @@ if (argc == 3) { - string path = maybe_add_default_load_path (argv[1]); - - string fname = search_path_for_file (path, argv[2]); + string fname = search_path_for_file (argv[1], argv[2]); if (fname.empty ()) retval = Matrix (); at @ -642,6 +640,13 @@ } return pref; +} + +DEFUN (load_path_default, args, , + " load_path_default ()") +{ + octave_value_list retval = Vload_path_default; + return retval; } /* --Multipart_Fri_Oct_23_09:40:42_1998-1--