Re: difference between path / pathdef

[John W. Eaton]

On 16-Jan-2008, Ben Abbott wrote:

| I've made the changes and built with the result. Everything appears to  
| be working.

I applied your patch and checked it in with the following additional
changes.

Thanks,

jwe


scripts/ChangeLog:

2008-01-17  John W. Eaton  <address@hidden>

        * path/pathdef.m: Use fullfile instead of concatenating with filesep.
        * path/__extractpath__.m, path/savepath.m: Use unwind_protect to
        avoid possible file descriptor leak.

 
Index: scripts/path/__extractpath__.m
===================================================================
RCS file: /cvs/octave/scripts/path/__extractpath__.m,v
retrieving revision 1.2
diff -u -u -r1.2 __extractpath__.m
--- scripts/path/__extractpath__.m      17 Jan 2008 07:50:33 -0000      1.2
+++ scripts/path/__extractpath__.m      17 Jan 2008 08:09:31 -0000
@@ -30,7 +30,9 @@
 
 function specifiedpath = __extractpath__ (savefile)
 
-  ## The majority of this code was borrowed from savepath.m
+  ## The majority of this code was borrowed from savepath.m.
+  ## FIXME -- is there some way to share the common parts instead of
+  ## duplicating?
 
   beginstring = "## Begin savepath auto-created section, do not edit";
   endstring   = "## End savepath auto-created section";
@@ -39,8 +41,8 @@
     savefile = tilde_expand ("~/.octaverc");
   endif
 
-  ## parse the file if it exists to see if we should replace a section
-  ## or create a section
+  ## Parse the file if it exists to see if we should replace a section
+  ## or create a section.
   startline = 0;
   endline = 0;
   filelines = {};
@@ -50,30 +52,34 @@
     if (fid < 0)
       error ("__extractpath__: could not open savefile, %s: %s", savefile, 
msg);
     endif
-    linenum = 0;
-    while (linenum >= 0)
-      result = fgetl (fid);
-      if (isnumeric (result))
-        ## end at the end of file
-        linenum = -1;
-      else
-        linenum = linenum + 1;
-        filelines{linenum} = result;
-        ## find the first and last lines if they exist in the file
-        if (strcmp (result, beginstring))
-          startline = linenum+1;
-        elseif (strcmp (result, endstring))
-          endline = linenum-1;
-        endif
+    unwind_protect
+      linenum = 0;
+      while (linenum >= 0)
+       result = fgetl (fid);
+       if (isnumeric (result))
+         ## End at the end of file.
+         linenum = -1;
+       else
+         linenum++;
+         filelines{linenum} = result;
+         ## Find the first and last lines if they exist in the file.
+         if (strcmp (result, beginstring))
+           startline = linenum + 1;
+         elseif (strcmp (result, endstring))
+           endline = linenum - 1;
+         endif
+       endif
+      endwhile
+    unwind_protect_cleanup
+      closeread = fclose (fid);
+      if (closeread < 0)
+       error ("savepath: could not close savefile after reading, %s",
+              savefile);
       endif
-    endwhile
-    closeread = fclose (fid);
-    if (closeread < 0)
-      error ("savepath: could not close savefile after reading, %s", savefile);
-    endif
+    end_unwind_protect
   endif
 
-  ## extract the path specifiation
+  ## Extract the path specifiation.
   if (startline > endline || (startline > 0 && endline == 0))
     error ("savepath: unable to parse file, %s", savefile);
   elseif startline > 0
Index: scripts/path/pathdef.m
===================================================================
RCS file: /cvs/octave/scripts/path/pathdef.m,v
retrieving revision 1.2
diff -u -u -r1.2 pathdef.m
--- scripts/path/pathdef.m      17 Jan 2008 07:50:33 -0000      1.2
+++ scripts/path/pathdef.m      17 Jan 2008 08:09:31 -0000
@@ -25,39 +25,42 @@
 ## @enumerate
 ## @item @file{~/.octaverc}
 ## @item @file{<octave-home>/.../<version>/m/startup/octaverc}
-## @item Octave"s path prior to changes by any octaverc.
+## @item Octave's path prior to changes by any octaverc.
 ## @end enumerate
 ## @seealso{path, addpath, rmpath, genpath, savepath, pathsep}
 ## @end deftypefn
 
 function val = pathdef ()
 
-  ## Use Octave"s orignal path as the default default.
+  ## Use Octave's orignal path as the default default.
   val = __pathorig__ ();
 
-  ## Locate the site octaverc file (is there a better way?).
+  ## Locate the site octaverc file.
   pathdir = octave_config_info ("localstartupfiledir");
-  site_octaverc = [pathdir, filesep, "octaverc"];
+  site_octaverc = fullfile (pathdir, "octaverc");
 
   ## locate the user ~\.octaverc file.
-  user_octaverc = ["~", filesep, ".octaverc"];
-  user_octaverc = sprintf ("~%s.octaverc", filesep);
+  user_octaverc = fullfile ("~", ".octaverc");
 
   ## Extract the specified paths from the site and user octaverc"s.
   site_pathscript = __extractpath__ (site_octaverc);
-  if exist (user_octaverc, "file")
+  if (exist (user_octaverc, "file"))
     user_pathscript = __extractpath__ (user_octaverc);
   else
     user_pathscript = "";
   endif
 
-  ## A path definition in the user octaverc has precedence over the site
-  if numel (user_pathscript)
+  ## A path definition in the user octaverc has precedence over the
+  ## site.
+
+  ## FIXME -- use a subfunction here to avoid code duplication?
+
+  if (numel (user_pathscript))
     try
       if (numel (user_pathscript) == 1)
-        n = strfind (user_pathscript{1},"'");
+        n = strfind (user_pathscript{1}, "'");
         if (numel(n) == 1)
-          n = strfind (user_pathscript{1},"""");
+          n = strfind (user_pathscript{1}, "\"");
         endif
         val = user_pathscript{1}(n(1):n(end));
       else
@@ -67,14 +70,14 @@
         path (presentpath);
       endif
     catch
-      warning ("pathdef: Path defined in `%s' produced an 
error.",user_octaverc)
+      warning ("pathdef: invalid path found in `%s'", user_octaverc);
     end_try_catch
-  elseif numel (site_pathscript)
+  elseif (numel (site_pathscript))
     try
       if (numel (site_pathscript) == 1)
-        n = strfind (site_pathscript{1},"'");
+        n = strfind (site_pathscript{1}, "'");
         if (numel(n) == 1)
-          n = strfind (site_pathscript{1},"""");
+          n = strfind (site_pathscript{1}, "\"");
         endif
         val = site_pathscript{1}(n(1):n(end));
       else
@@ -84,10 +87,8 @@
         path (presentpath);
       endif
     catch
-      warning ("pathdef: Path defined in `%s' produced an 
error.",site_octaverc)
+      warning ("pathdef: invalid path found in `%s'", site_octaverc);
     end_try_catch
   endif
 
 endfunction
-
-
Index: scripts/path/savepath.m
===================================================================
RCS file: /cvs/octave/scripts/path/savepath.m,v
retrieving revision 1.13
diff -u -u -r1.13 savepath.m
--- scripts/path/savepath.m     17 Jan 2008 07:50:33 -0000      1.13
+++ scripts/path/savepath.m     17 Jan 2008 08:09:31 -0000
@@ -50,27 +50,31 @@
     if (fid < 0)
       error ("savepath: could not open savefile, %s: %s", savefile, msg);
     endif
-    linenum = 0;
-    while (linenum >= 0)
-      result = fgetl (fid);
-      if (isnumeric (result))
-        ## end at the end of file
-        linenum = -1;
-      else
-        linenum = linenum + 1;
-        filelines{linenum} = result;
-        ## find the first and last lines if they exist in the file
-        if (strcmp (result, beginstring))
-          startline = linenum;
-        elseif (strcmp (result, endstring))
-          endline = linenum;
-        endif
+    unwind_protect
+      linenum = 0;
+      while (linenum >= 0)
+       result = fgetl (fid);
+       if (isnumeric (result))
+         ## end at the end of file
+         linenum = -1;
+       else
+         linenum = linenum + 1;
+         filelines{linenum} = result;
+         ## find the first and last lines if they exist in the file
+         if (strcmp (result, beginstring))
+           startline = linenum;
+         elseif (strcmp (result, endstring))
+           endline = linenum;
+         endif
+       endif
+      endwhile
+    unwind_protect_cleanup
+      closeread = fclose (fid);
+      if (closeread < 0)
+       error ("savepath: could not close savefile after reading, %s",
+              savefile);
       endif
-    endwhile
-    closeread = fclose (fid);
-    if (closeread < 0)
-      error ("savepath: could not close savefile after reading, %s", savefile);
-    endif
+    end_unwind_protect
   endif
 
   if (startline > endline || (startline > 0 && endline == 0))
@@ -103,24 +107,27 @@
   if (fid < 0)
     error ("savepath: unable to open file for writing, %s, %s", savefile, msg);
   endif
-  for i = 1:length (pre)
-    fprintf (fid, "%s\n", pre{i})
-  endfor
-
-  ## Use single quotes for PATH argument to avoid string escape
-  ## processing.
-  fprintf (fid, "%s\n  path ('%s');\n%s\n",
-          beginstring, path (), endstring);
-
-  for i = 1:length (post)
-    fprintf (fid, "%s\n", post{i});
-  endfor
-  closeread = fclose (fid);
-  if (closeread < 0)
-    error ("savepath: could not close savefile after writing, %s", savefile);
-  elseif (nargin == 0)
-    warning ("savepath: current path saved to %s",savefile)
-  endif
+  unwind_protect
+    for i = 1:length (pre)
+      fprintf (fid, "%s\n", pre{i})
+    endfor
+
+    ## Use single quotes for PATH argument to avoid string escape
+    ## processing.
+    fprintf (fid, "%s\n  path ('%s');\n%s\n",
+            beginstring, path (), endstring);
+
+    for i = 1:length (post)
+      fprintf (fid, "%s\n", post{i});
+    endfor
+  unwind_protect_cleanup
+    closeread = fclose (fid);
+    if (closeread < 0)
+      error ("savepath: could not close savefile after writing, %s", savefile);
+    elseif (nargin == 0)
+      warning ("savepath: current path saved to %s", savefile);
+    endif
+  end_unwind_protect
 
   retval = 0;
 
Index: scripts/startup/__finish__.m
===================================================================
RCS file: /cvs/octave/scripts/startup/__finish__.m,v
retrieving revision 1.1
diff -u -u -r1.1 __finish__.m
--- scripts/startup/__finish__.m        16 Jan 2008 06:31:23 -0000      1.1
+++ scripts/startup/__finish__.m        17 Jan 2008 08:09:31 -0000
@@ -30,7 +30,7 @@
 
 function __finish__ ()
 
-  if exist ('finish','file')
+  if (exist ("finish", "file"))
     ## No arg list here since finish might be a script.
     finish;
   endif
Index: scripts/startup/main-rcfile
===================================================================
RCS file: /cvs/octave/scripts/startup/main-rcfile,v
retrieving revision 1.6
diff -u -u -r1.6 main-rcfile
--- scripts/startup/main-rcfile 17 Jan 2008 07:50:33 -0000      1.6
+++ scripts/startup/main-rcfile 17 Jan 2008 08:09:31 -0000
@@ -19,5 +19,4 @@
 
 pkg ("load", "auto");
 
-atexit ('__finish__');
-
+atexit ("__finish__");