Re: [Changeset] imfinfo

[John W. Eaton]

On 25-Sep-2008, address@hidden wrote:

| Quoting "John W. Eaton" <address@hidden>:
| 
| > On 24-Sep-2008, Søren Hauberg wrote:
| >
| > |   The attached changeset implement the 'imfinfo' function for reading
| > | header information from image files. I'm not sure if this function is
| > | part of core Matlab or the Image Processing Toolbox. But it was fairly
| > | easy to add to core Octave, so that's what I'm proposing.
| >
| > I applied the patch with a few changes, plus I added ChangeLog
| > entries.  I simplified the code you wrote to download a file given a
| > URL by using the urlwrite function instead of urlread and fwrite,
| 
| Thanks -- I thought 'urlwrite' allowed you to write data to some given  
| url (i.e. save data on some server), but I was mistaken. I do think  
| there's a problem with the changes you made. If a file is downloaded  
| from some url, it looks like it won't be deleted when we're done with  
| it. I mean, shouldn't there be an
| 
|    if (file_was_downloaded)
|      unlink (fn)
|    endif
| 
| at the end of the function?

I made the following changes.

Thanks,

jwe

# HG changeset patch
# User John W. Eaton <address@hidden>
# Date 1222449493 14400
# Node ID 2b48deec1aa2e0a0c54e6516396a2892744e2718
# Parent  3725f819b5b3c023e6c73a98c7a9bdc7aec88aa4
imfinfo.m: delete temporary files

diff --git a/scripts/ChangeLog b/scripts/ChangeLog
--- a/scripts/ChangeLog
+++ b/scripts/ChangeLog
@@ -1,3 +1,7 @@
+2008-09-26  John W. Eaton  <address@hidden>
+
+       * image/imfinfo.m: Delete temporary file.
+
 2008-09-25  Søren Hauberg  <address@hidden>
 
        * image/imread.m, image/imwrite.m: Doc fix.
diff --git a/scripts/image/imfinfo.m b/scripts/image/imfinfo.m
--- a/scripts/image/imfinfo.m
+++ b/scripts/image/imfinfo.m
@@ -95,26 +95,46 @@
 
   filename = tilde_expand (filename);
 
-  fn = file_in_path (IMAGE_PATH, filename);
-  if (isempty (fn))
-    ## Couldn't find file. See if it's an URL.
-    tmp = tmpnam ();
+  delete_file = false;
 
-    [fn, status, msg] = urlwrite (filename, tmp);
+  unwind_protect
 
-    if (! status)
-      error ("imfinfo: cannot find %s", filename);
+    fn = file_in_path (IMAGE_PATH, filename);
+
+    if (isempty (fn))
+
+      ## Couldn't find file. See if it's an URL.
+
+      tmp = tmpnam ();
+
+      [fn, status, msg] = urlwrite (filename, tmp);
+
+      if (! status)
+       error ("imfinfo: cannot find %s", filename);
+      endif
+
+      if (! isempty (fn))
+       delete_file = true;
+      endif
+
     endif
-  endif
 
-  [statinfo, err, msg] = stat (fn);
-  if (err != 0)
-    error ("imfinfo: error reading '%s': %s", fn, msg);
-  endif
+    [statinfo, err, msg] = stat (fn);
+    if (err != 0)
+      error ("imfinfo: error reading '%s': %s", fn, msg);
+    endif
 
-  time_stamp = strftime ("%e-%b-%Y %H:%M:%S", localtime (statinfo.mtime));
+    time_stamp = strftime ("%e-%b-%Y %H:%M:%S", localtime (statinfo.mtime));
   
-  info = __magick_finfo__ (filename);
-  info.FileModDate = time_stamp;
+    info = __magick_finfo__ (fn);
+    info.FileModDate = time_stamp;
+
+  unwind_protect_cleanup
+
+    if (delete_file)
+      unlink (fn);
+    endif
+
+  end_unwind_protect
 
 endfunction

# HG changeset patch
# User John W. Eaton <address@hidden>
# Date 1222449402 14400
# Node ID 3725f819b5b3c023e6c73a98c7a9bdc7aec88aa4
# Parent  283989f2da9ba525c7eca8c77051220bd295fade
urlwrite.cc (Furlwrite): delete files we create if download fails

diff --git a/src/ChangeLog b/src/ChangeLog
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,10 @@
+2008-09-26  John W. Eaton  <address@hidden>
+
+       * DLD-FUNCTIONS/urlwrite.cc (urlwrite_cleanup_file) New function.
+       (urlwrite_delete_file, urlwrite_filename): New static variables.
+       (Furlwrite): Only return filename if urlget succeeds.  Use
+       unwind_protect to delete files we create if download fails.
+
 2008-09-26  Jaroslav Hajek  <address@hidden>
 
        * ov-null-mat.h: New header file.
diff --git a/src/DLD-FUNCTIONS/urlwrite.cc b/src/DLD-FUNCTIONS/urlwrite.cc
--- a/src/DLD-FUNCTIONS/urlwrite.cc
+++ b/src/DLD-FUNCTIONS/urlwrite.cc
@@ -32,6 +32,8 @@
 #include <fstream>
 #include <iomanip>
 
+#include "file-ops.h"
+#include "file-stat.h"
 #include "oct-env.h"
 
 #include "defun-dld.h"
@@ -39,6 +41,7 @@
 #include "oct-obj.h"
 #include "ov-cell.h"
 #include "pager.h"
+#include "unwind-prot.h"
 
 #if defined (HAVE_CURL)
 
@@ -185,6 +188,17 @@
 
 #endif
 
+static bool urlwrite_delete_file;
+
+static std::string urlwrite_filename;
+
+static void
+urlwrite_cleanup_file (void *)
+{
+  if (urlwrite_delete_file)
+    file_ops::unlink (urlwrite_filename);
+}
+
 DEFUN_DLD (urlwrite, args, nargout,
   "-*- texinfo -*-\n\
 @deftypefn {Loadable Function} {} urlwrite (@var{URL}, @var{localfile})\n\
@@ -250,6 +264,8 @@
   // name to store the file if download is succesful
   std::string filename = args(1).string_value();
 
+  urlwrite_filename = filename;
+
   if (error_state)
     {
       error ("urlwrite: localfile must be a character string");
@@ -291,6 +307,14 @@
        }
     }
 
+  // The file should only be deleted if it doesn't initially exist, we
+  // create it, and the download fails.  We use unwind_protect to do
+  // it so that the deletion happens no matter how we exit the function.
+
+  file_stat fs (filename);
+
+  urlwrite_delete_file = ! fs.exists ();
+
   std::ofstream ofile (filename.c_str(), std::ios::out | std::ios::binary);
 
   if (! ofile.is_open ())
@@ -299,15 +323,30 @@
       return retval;
     }
 
+  unwind_protect::add (urlwrite_cleanup_file);
+
   CURLcode res = urlget (url, method, param, ofile);
 
   ofile.close ();
 
+  urlwrite_delete_file = (res != CURLE_OK);
+
+  unwind_protect::run ();
+
   if (nargout > 0)
     {
-      retval(0) = octave_env::make_absolute (filename, octave_env::getcwd ());
-      retval(1) = res == CURLE_OK;
-      retval(2) = std::string (res == CURLE_OK ? "" : curl_easy_strerror 
(res));
+      if (res == CURLE_OK)
+       {
+         retval(2) = std::string ();
+         retval(1) = true;
+         retval(0) = octave_env::make_absolute (filename, octave_env::getcwd 
());
+       }
+      else
+       {
+         retval(2) = std::string (curl_easy_strerror (res));
+         retval(1) = false;
+         retval(0) = std::string ();
+       }
     }
 
   if (nargout < 2 && res != CURLE_OK)