bugGNU Octave - Bugs: bug #32158, Error of deleting a file file path...

 
 

bug #32158: Error of deleting a file file path which begins with '\' on MinGW

Submitted by:  Tatsuro MATSUOKA <tmacchant>
Submitted on:  Mon 17 Jan 2011 09:34:32 PM UTC  
 
Category:  Interpreter Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Incorrect Result
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  Closed
Release:  dev Operating System:  Microsoft Windows

Add a New Comment (Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

( Jump to the original submission)

Mon 31 Jan 2011 11:52:40 AM UTC, comment #18: 

OK, I'm closing this report and also

https://savannah.gnu.org/bugs/?31924

which I think was due to the same problem.

John W. Eaton <jwe>
Project Administrator
Mon 31 Jan 2011 11:47:35 AM UTC, comment #17: 

Apparently, since there are no stray files left in the temporary directory.

Benjamin Lindner <lindnerben>
Mon 31 Jan 2011 10:50:51 AM UTC, comment #16: 

Thanks.  Are temporary files created by help also created and deleted properly now?

John W. Eaton <jwe>
Project Administrator
Mon 31 Jan 2011 10:41:29 AM UTC, comment #15: 

Yes, now P_tmpdir reurns the same as getenv("TEMP")

Benjamin Lindner <lindnerben>
Mon 31 Jan 2011 08:23:21 AM UTC, comment #14: 

Oops, I just wasn't seeing that.

OK, I checked in the following change:

http://hg.savannah.gnu.org/hgweb/octave/rev/0f70c5db58c3

Does this work for you?

John W. Eaton <jwe>
Project Administrator
Mon 31 Jan 2011 07:49:14 AM UTC, comment #13: 

P_tmpdir returns an empty string, and I have both TEMP and TMP set in the environment (i.e. getenv("TEMP") and getenv("TMP") both return non-empty strings.
Which I don't quite understand looking at your patch...

Ah, I see the flaw in the patch:
the return value in get_P_tmpdir "retval" is not properly assigned. It's assigned in the else clause whereby if the else clause is not executed (which it isn't, as P_tmpdir is "\\", so the code which checks for TEMP and TMP is executed) the still empty retval is returned.

Benjamin Lindner <lindnerben>
Thu 27 Jan 2011 08:03:09 PM UTC, comment #12: 

Yes, Benjamin, please let me know what value P_tmpdir has on your system.  Also, do you have TEMP or TMP set in your environment?

John W. Eaton <jwe>
Project Administrator
Thu 27 Jan 2011 05:59:27 PM UTC, comment #11: 

Benjamin
********
I applied your proposed patch and these errors disappeared.
Also there are no stray files in the root directory of the current
drive created any more.
So I'd call this problem solved.
***********

Did you check the value P_tmpdir. For me it is empty. Thus temporary file may be created in the present working directory and deleting process will be successful.  However, I do not think that the fact that P_tmpdir is empty is not what John hope.

Tatsuro MATSUOKA <tmacchant>
Thu 27 Jan 2011 05:53:39 PM UTC, comment #10: 

No, I was hoping someone would try it and report whether this solved
the problem with deleting temporary files created by the help command
before I committed the change.

jwe

*********************
I applied your proposed patch and these errors disappeared.
Also there are no stray files in the root directory of the current
drive created any more.
So I'd call this problem solved.

benjamin

Tatsuro MATSUOKA <tmacchant>
Wed 26 Jan 2011 07:13:20 AM UTC, comment #9: 

I don't see how P_tmpdir can be empty if you applied my patch.

As attached to this bug report, the key parts of this patch are:

diff --git a/src/file-io.cc b/src/file-io.cc
--- a/src/file-io.cc
+++ b/src/file-io.cc
@@ -2122,10 +2122,6 @@
   return retval;
}

-#if ! defined (P_tmpdir)
-#define P_tmpdir "/tmp"
-#endif
-
DEFUNX ("P_tmpdir", FP_tmpdir, args, ,
   "-*- texinfo -*-\n\
@deftypefn {Built-in Function} {} P_tmpdir ()\n\
@@ -2138,7 +2134,7 @@
   int nargin = args.length ();

   if (nargin == 0)
-    retval = P_tmpdir;
+    retval = get_P_tmpdir ();
   else
     print_usage ();

diff --git a/src/sysdep.cc b/src/sysdep.cc
--- a/src/sysdep.cc
+++ b/src/sysdep.cc
@@ -470,6 +470,45 @@
   return c;
}

+std::string
+get_P_tmpdir (void)
+{
+#if defined (__WIN32__) && ! defined (_POSIX_VERSION)
+
+  std::string retval;
+
+  // Apparently some versions of MinGW and MSVC either don't define
+  // P_tmpdir, or they define it to a single backslash, neither of which
+  // is particularly helpful.
+
+  std::string tmp = P_tmpdir;
+
+  if (tmp.empty () || tmp == "\\")
+    {
+      tmp = octave_env::getenv ("TEMP");
+
+      if (tmp.empty ())
+        tmp = octave_env::getenv ("TMP");
+
+      if (tmp.empty ())
+        tmp = "c:\\temp";
+    }
+  else
+    retval = tmp;
+
+  return retval;
+
+#elif defined (P_tmpdir)
+
+  return P_tmpdir;
+
+#else
+
+  return "/tmp";
+
+#endif
+}
+

So it makes Octave's P_tmpdir function always call the new get_P_tmpdir function, and on Windows systems, that should never return an empty string.  If the macro P_tmpdir is defined to "" or "\\", then it looks at TEMP and then TMP in the environment.  If neither of those is set (or they are empty) then it returns "c:\\temp".

Can you run Octave under gdb, set a breakpoint in get_P_tmpdir, then call P_tmpdir and step through get_P_tmpdir to see whether this is happening as I expect it should?

John W. Eaton <jwe>
Project Administrator
Wed 26 Jan 2011 06:22:28 AM UTC, comment #8: 

Hello

I remember the build in the university PC used your patch.
However, I cannot remove my local patch because I cannot build without them.

Therefore the result is the same as before

octave:1> P_tmpdir
ans =

P_tmpdir is empty.  Thus the temporary files are made in the present working directory but they are deleted after they are used.

Tatsuro MATSUOKA <tmacchant>
Thu 20 Jan 2011 08:30:47 AM UTC, comment #7: 

Please test the current sources with my patch applied but without any of your local patches applied.

I don't see how P_tmpdir could be empty after my latest changes have been made as it is initialized by the new function get_P_tmpdir.  I don't see any way that that function can return an empty string, or a single backslash.

John W. Eaton <jwe>
Project Administrator
Wed 19 Jan 2011 09:34:55 PM UTC, comment #6: 

>P_tmpdir is still empty after applying the patch?

Before applying your patch P_tmpdir is '\' (backslash) but not empty. The unlink on mingw fails to delte '\(path name of file)
This causes fails to delete temporary file like octave-help-xxxxx.

Ooops!  I remember!

http://octave.1599824.n4.nabble.com/quot-Error-unlink-is-not-a-member-of-gnulib-quot-on-the-recent-check-out-of-development-branch-was-R-td2252605.html#a2253063

http://octave.1599824.n4.nabble.com/compile-error-on-oct-env-cc-in-building-octave-3-3-52-on-MinGW32-td2549610.html#a2549977

So I am using local patch.

The below is my local patch
*******************************
--- a/src/oct-hist.cc 2010-03-16 17:25:40.703125000 +0900
+++ b/src/oct-hist.cc 2010-07-28 12:02:50.798004100 +0900
@@ -423,7 +423,7 @@
 static void
 unlink_cleanup (const char *file)
 {
-  gnulib::unlink (file);
+  unlink (file); //gnulib::unlink (file);
 }

 static void
*********************************
--- octave/liboctave/oct-env.cc 2010-11-12 07:31:06 +0900
+++ octave-work/liboctave/oct-env.cc 2010-11-22 12:03:04 +0900
@@ -447,7 +447,7 @@
     {
       char hostname[1024];

-      int status = gnulib::gethostname (hostname, 1023);
+      int status = gethostname (hostname, 1023); //gnulib::gethostname (hostname, 1023);

       host_name = (status < 0) ? "unknown" : hostname;
     }
**********************************

I do not test the current gnulib solve the above issues.

Is it better to test, the below are fixed using current gnulib source?  If the bug will not be fixed, is it better register this issue to the bug tracker?

Tatsuro MATSUOKA <tmacchant>
Wed 19 Jan 2011 05:03:25 PM UTC, comment #5: 

>Do you still have your patch applied?

No. I did not applied.
I guess that P_tmpdir is now empty so that temporally file is made in the current directory in preparing help information so that unlink can delete it.

I'm now at home so that I cannot use rebuild one with the patch No.file #22435 and cannot report further at present.  Please wait until I will come to the University.

My patch only affects to delete,m so that it may not affect P_tmpdir behavior.

Tatsuro MATSUOKA <tmacchant>
Wed 19 Jan 2011 10:11:01 AM UTC, comment #4: 

P_tmpdir is still empty after applying the patch?  That's not what I intended for it to do.  Are you sure that it is empty?  Do you still have your patch applied?

John W. Eaton <jwe>
Project Administrator
Wed 19 Jan 2011 08:59:59 AM UTC, comment #3: 

I have applied your patch and built octave

The results are
************************
octave.exe:1> P_tmpdir
ans =

The temporary file error using help command disappeared without my patch to delete.m.

Thanks!!

Tatsuro MATSUOKA <tmacchant>
Tue 18 Jan 2011 10:18:30 AM UTC, comment #2: 

Does the attached patch avoid the problem?

(file #22435)

John W. Eaton <jwe>
Project Administrator
Mon 17 Jan 2011 09:58:22 PM UTC, comment #1: 

>Why are temporary file names like '\octave-help-xxxxxx' being generated?


I do not find out the reason.

>Are those valid filenames on Windows systems?

Yes it is.

Below the pacth
***************
      for i = 1:length (files)
        file = files{i};
        [err, msg] = unlink (file);
        if (err)
          warning ("delete: %s: %s", file, msg);
        endif
***************

'unlink' cannot detele '\octave-help-xxxx' but can delete 'C:\octave-help-xxxx' so that I added current drive name like 'C:'.

>What is the value of P_tmpdir in your version of Octave

*************
debug> P_tmpdir
ans = \
***************

My windows setting for environmental variable tmp is
*******************
octave:2> getenv('tmp')
ans = C:\DOCUME~1\Tatsu\LOCALS~1\Temp

If unlink can delete a file patch name of which begins from '\', this patch is not required.

Tatsuro MATSUOKA <tmacchant>
Mon 17 Jan 2011 09:34:32 PM UTC, original submission:  

I move this topic from patch tracker to here according to John's suggestion.
(https://savannah.gnu.org/patch/?7438)

My submission****************
I submit a change set to fix error of deleting a file file path which begins with '\' on MinGW (dev source).  Without the patch, octave often fail to delete temporary files with path like '\octave-help-xxxxx' 

John's reply ****************
I wonder if you are maybe fixing the wrong thing. Why are temporary file names like '\octave-help-xxxxxx' being generated? Are those valid filenames on Windows systems? If so, then why is it possible to use them for some purposes (opening and writing to them, apparently) but not others (deleting)? What is the value of P_tmpdir in your version of Octave? How does the help function generate temporary file names?

Tatsuro MATSUOKA <tmacchant>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #22435:  diffs.txt added by jwe (2KiB - text/plain)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by lindnerben (Posted a comment)
  • -email is unavailable- added by tmacchant
  • -email is unavailable- added by jwe (Updated the item)
  • -email is unavailable- added by tmacchant (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can add your encouragement to it.
    This task has 0 encouragements so far.

    Only project members can vote.

     

     

     

    Follow 14 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2011-01-31 jwe StatusReady For Test => Fixed
        Open/ClosedOpen => Closed
        SummaryError of deleting a file file path which begins with \'\\\' on MinGW => Error of deleting a file file path which begins with '\' on MinGW
    2011-01-31 jwe SummaryError of deleting a file file path which begins with \'\\\' on MinGW => Error of deleting a file file path which begins with '\' on MinGW
    2011-01-31 jwe SummaryError of deleting a file file path which begins with \'\\\' on MinGW => Error of deleting a file file path which begins with '\' on MinGW
    2011-01-27 jwe SummaryError of deleting a file file path which begins with \'\\\' on MinGW => Error of deleting a file file path which begins with '\' on MinGW
    2011-01-27 tmacchant Carbon-Copy- => Added "benjamin lindner" <bjmldn@gmail.com>
    2011-01-26 jwe SummaryError of deleting a file file path which begins with \'\\\' on MinGW => Error of deleting a file file path which begins with '\' on MinGW
    2011-01-20 jwe SummaryError of deleting a file file path which begins with \'\\\' on MinGW => Error of deleting a file file path which begins with '\' on MinGW
    2011-01-19 jwe SummaryError of deleting a file file path which begins with \'\\\' on MinGW => Error of deleting a file file path which begins with '\' on MinGW
    2011-01-18 jwe Attached File- => Added diffs.txt, #22435
        StatusNone => Ready For Test
        SummaryError of deleting a file file path which begins with \'\\\' on MinGW => Error of deleting a file file path which begins with '\' on MinGW
    2011-01-17 tmacchant Attached File- => Added delete.m.diff, #22432

    Back to the top


    Powered by Savane 3.5