Re: [Patch libfortran] PR 51803 getcwd() failure
On Wed, Jan 11, 2012 at 14:55, Tobias Burnus bur...@net-b.de wrote:
Dear all,
this is a follow up patch, which I think provides a better handling if
either getcwd fails or is not availble - or if the pathname in argv[0]
already is an absolute patch, in which case concatenating
current-working-directory + '/' + argv[0] does not really make sense.
Build on x86-64-linux.
OK for the trunk?
Committed the patch below, which implements Tobias' suggestion, as obvious.
Index: runtime/main.c
===
--- runtime/main.c (revision 183121)
+++ runtime/main.c (working copy)
@@ -124,12 +124,17 @@ store_exe_path (const char * argv0)
#ifdef HAVE_GETCWD
cwd = getcwd (buf, sizeof (buf));
- if (!cwd)
-cwd = .;
#else
- cwd = .;
+ cwd = NULL;
#endif
+ if (!cwd)
+{
+ exe_path = argv0;
+ please_free_exe_path_when_done = 0;
+ return;
+}
+
/* exe_path will be cwd + / + argv[0] + \0. This will not work
if the executable is not in the cwd, but at this point we're out
of better ideas. */
Index: ChangeLog
===
--- ChangeLog (revision 183121)
+++ ChangeLog (working copy)
@@ -1,3 +1,10 @@
+2012-01-12 Janne Blomqvist j...@gcc.gnu.org
+ Tobias Burnus bur...@net-b.de
+
+ PR libfortran/51803
+ * runtime/main.c (store_exe_path): Avoid malloc if getcwd fails or
+ is not available.
+
2012-01-11 Tobias Burnus bur...@net-b.de
* runtime/main.c (store_exe_path): Fix absolute path
@@ -5,6 +12,7 @@
2012-01-11 Janne Blomqvist j...@gcc.gnu.org
Mike Stump mikest...@comcast.net
+
PR libfortran/51803
* runtime/main.c (store_exe_path): Handle getcwd failure and lack
of the function better.
--
Janne Blomqvist
Re: [Patch libfortran] PR 51803 getcwd() failure
Hi Janne,
On 01/11/2012 08:37 AM, Janne Blomqvist wrote:
Index: runtime/main.c
===
--- runtime/main.c (revision 183089)
+++ runtime/main.c (working copy)
@@ -116,8 +116,10 @@ store_exe_path (const char * argv0)
memset (buf, 0, sizeof (buf));
#ifdef HAVE_GETCWD
cwd = getcwd (buf, sizeof (buf));
+ if (!cwd)
+cwd = .;
#else
- cwd = ;
+ cwd = .;
#endif
I had preferred a patch like the following.
Tobias
--- a/libgfortran/runtime/main.c
+++ b/libgfortran/runtime/main.c
@@ -117,9 +117,16 @@ store_exe_path (const char * argv0)
#ifdef HAVE_GETCWD
cwd = getcwd (buf, sizeof (buf));
#else
- cwd = ;
+ cwd = NULL;
#endif
+ if (cwd == NULL)
+{
+ exe_path = argv0;
+ please_free_exe_path_when_done = 0;
+ return;
+}
+
/* exe_path will be cwd + / + argv[0] + \0. This will not work
if the executable is not in the cwd, but at this point we're out
of better ideas. */
Re: [Patch libfortran] PR 51803 getcwd() failure
Dear all,
this is a follow up patch, which I think provides a better handling if
either getcwd fails or is not availble - or if the pathname in argv[0]
already is an absolute patch, in which case concatenating
current-working-directory + '/' + argv[0] does not really make sense.
Build on x86-64-linux.
OK for the trunk?
Tobias
2012-01-11 Tobias Burnus bur...@net-b.de
PR fortran/51803
* runtime/main.c (store_exe_path): Handle a failure of
getcwd and take absolute pathnames into account.
Index: libgfortran/runtime/main.c
===
--- libgfortran/runtime/main.c (revision 183092)
+++ libgfortran/runtime/main.c (working copy)
@@ -116,12 +116,17 @@ store_exe_path (const char * argv0)
memset (buf, 0, sizeof (buf));
#ifdef HAVE_GETCWD
cwd = getcwd (buf, sizeof (buf));
- if (!cwd)
-cwd = .;
#else
- cwd = .;
+ cwd = NULL;
#endif
+ if (!cwd || argv0[0] == DIR_SEPARATOR)
+{
+ exe_path = argv0;
+ please_free_exe_path_when_done = 0;
+ return;
+}
+
/* exe_path will be cwd + / + argv[0] + \0. This will not work
if the executable is not in the cwd, but at this point we're out
of better ideas. */
Re: [Patch libfortran] PR 51803 getcwd() failure
On Wed, Jan 11, 2012 at 14:55, Tobias Burnus bur...@net-b.de wrote:
Dear all,
this is a follow up patch, which I think provides a better handling if
either getcwd fails or is not availble - or if the pathname in argv[0]
already is an absolute patch, in which case concatenating
current-working-directory + '/' + argv[0] does not really make sense.
Checking for an absolute path is already done a few lines up. So if
you prefer the kind of approach that you have in your patch, IMHO a
more correct patch would be
Index: main.c
===
--- main.c (revision 183091)
+++ main.c (working copy)
@@ -106,22 +106,26 @@
#endif
/* On the simulator argv is not set. */
- if (argv0 == NULL || argv0[0] == '/')
+ if (argv0 == NULL || argv0[0] == DIR_SEPARATOR)
{
exe_path = argv0;
please_free_exe_path_when_done = 0;
return;
}
- memset (buf, 0, sizeof (buf));
#ifdef HAVE_GETCWD
cwd = getcwd (buf, sizeof (buf));
- if (!cwd)
-cwd = .;
#else
- cwd = .;
+ cwd = NULL;
#endif
+ if (!cwd)
+{
+ exe_path = argv0;
+ please_free_exe_path_when_done = 0;
+ return;
+}
+
/* exe_path will be cwd + / + argv[0] + \0. This will not work
if the executable is not in the cwd, but at this point we're out
of better ideas. */
Also, I removed the memset() call as superfluous; getcwd() makes sure
that the buffer is null terminated or it will return NULL instead of a
pointer to the string.
For my part this fixed patch would be Ok.
--
Janne Blomqvist
Re: [Patch libfortran] PR 51803 getcwd() failure
On 01/11/2012 02:08 PM, Janne Blomqvist wrote:
Checking for an absolute path is already done a few lines up. So if
you prefer the kind of approach that you have in your patch, IMHO a
more correct patch would be
I had a quick chat with Kai and decided to leave the lower part as is.
However, I realized that the check for an absolute path is not correct
for Windows. With the help of Kai I came up with the attached version.
OK for the trunk?
Tobias
2012-01-11 Tobias Burnus bur...@net-b.de
* runtime/main.c (store_exe_path): Fix absolute path
detection for Windows.
Index: libgfortran/runtime/main.c
===
--- libgfortran/runtime/main.c (revision 183093)
+++ libgfortran/runtime/main.c (working copy)
@@ -105,15 +105,22 @@ store_exe_path (const char * argv0)
}
#endif
- /* On the simulator argv is not set. */
- if (argv0 == NULL || argv0[0] == '/')
+ /* If the path is absolute or on an simulator where argv is not set. */
+#ifdef __MINGW32__
+ if (argv0 == NULL
+ || ('A' = argv0[0] argv0[0] = 'Z' argv0[1] == ':')
+ || ('a' = argv0[0] argv0[0] = 'z' argv0[1] == ':')
+ || (argv0[0] == '/' argv0[1] == '/')
+ || (argv0[0] == '\\' argv0[1] == '\\'))
+#else
+ if (argv0 == NULL || argv0[0] == DIR_SEPARATOR)
+#endif
{
exe_path = argv0;
please_free_exe_path_when_done = 0;
return;
}
- memset (buf, 0, sizeof (buf));
#ifdef HAVE_GETCWD
cwd = getcwd (buf, sizeof (buf));
if (!cwd)
Re: [Patch libfortran] PR 51803 getcwd() failure
Same patch with a minor update: I changed cwd from char * to const
char * as I spotted a compile time warning for
cwd = .;
which was along the lines that by the assignment the const qualifier is
lost.
Too bad that we cannot enable -Werror for libgfortran.
On 01/11/2012 03:04 PM, Tobias Burnus wrote:
I had a quick chat with Kai and decided to leave the lower part as is.
However, I realized that the check for an absolute path is not correct
for Windows. With the help of Kai I came up with the attached version.
OK for the trunk?
2012-01-11 Tobias Burnus bur...@net-b.de
* runtime/main.c (store_exe_path): Fix absolute path
detection for Windows.
Index: libgfortran/runtime/main.c
===
--- libgfortran/runtime/main.c (revision 183093)
+++ libgfortran/runtime/main.c (working copy)
@@ -86,7 +86,8 @@ store_exe_path (const char * argv0)
#define DIR_SEPARATOR '/'
#endif
- char buf[PATH_MAX], *cwd, *path;
+ char buf[PATH_MAX], *path;
+ const char *cwd;
/* This can only happen if store_exe_path is called multiple times. */
if (please_free_exe_path_when_done)
@@ -105,15 +106,22 @@ store_exe_path (const char * argv0)
}
#endif
- /* On the simulator argv is not set. */
- if (argv0 == NULL || argv0[0] == '/')
+ /* If the path is absolute or on an simulator where argv is not set. */
+#ifdef __MINGW32__
+ if (argv0 == NULL
+ || ('A' = argv0[0] argv0[0] = 'Z' argv0[1] == ':')
+ || ('a' = argv0[0] argv0[0] = 'z' argv0[1] == ':')
+ || (argv0[0] == '/' argv0[1] == '/')
+ || (argv0[0] == '\\' argv0[1] == '\\'))
+#else
+ if (argv0 == NULL || argv0[0] == DIR_SEPARATOR)
+#endif
{
exe_path = argv0;
please_free_exe_path_when_done = 0;
return;
}
- memset (buf, 0, sizeof (buf));
#ifdef HAVE_GETCWD
cwd = getcwd (buf, sizeof (buf));
if (!cwd)
Re: [Patch libfortran] PR 51803 getcwd() failure
On Wed, Jan 11, 2012 at 16:04, Tobias Burnus bur...@net-b.de wrote: On 01/11/2012 02:08 PM, Janne Blomqvist wrote: Checking for an absolute path is already done a few lines up. So if you prefer the kind of approach that you have in your patch, IMHO a more correct patch would be I had a quick chat with Kai and decided to leave the lower part as is. However, I realized that the check for an absolute path is not correct for Windows. With the help of Kai I came up with the attached version. OK for the trunk? With the minor change s/an simulator/a simulator/ in the comment, Ok. In practice, I don't think this matters at the moment, since the only place where exe_path is used is when building the argument list for addr2line in the backtrace generation. Which we don't do on Windows anyway, since windows lacks fork, exec, and pipe which we require to launch the subprocess. Making cwd const char* is also Ok. -- Janne Blomqvist
[Patch libfortran] PR 51803 getcwd() failure
Hi, I committed the attached patch as obvious to trunk after the RM considered it OK in the PR. Index: runtime/main.c === --- runtime/main.c (revision 183089) +++ runtime/main.c (working copy) @@ -116,8 +116,10 @@ store_exe_path (const char * argv0) memset (buf, 0, sizeof (buf)); #ifdef HAVE_GETCWD cwd = getcwd (buf, sizeof (buf)); + if (!cwd) +cwd = .; #else - cwd = ; + cwd = .; #endif /* exe_path will be cwd + / + argv[0] + \0. This will not work Index: ChangeLog === --- ChangeLog (revision 183089) +++ ChangeLog (working copy) @@ -1,3 +1,9 @@ +2012-01-11 Janne Blomqvist j...@gcc.gnu.org +Mike Stump mikest...@comcast.net + PR libfortran/51803 + * runtime/main.c (store_exe_path): Handle getcwd failure and lack + of the function better. + 2012-01-10 Tobias Burnus bur...@net-b.de PR fortran/51197 -- Janne Blomqvist
