Re: [PATCH] allow empty string as argument to -Map

[Nick Clifton <nickc@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 162 bytes --]

Hi Fangrui, Hi Rasmus,

   Any comments on the attached patch ?

   It implements the basename and % character changes as previously discussed.

Cheers
   Nick



[-- Attachment #2: ld-map-directory.patch --]
[-- Type: text/x-patch, Size: 6924 bytes --]

diff --git a/ld/ld.texi b/ld/ld.texi
index cf8f05c39e..9b74b893c1 100644
--- a/ld/ld.texi
+++ b/ld/ld.texi
@@ -1866,10 +1866,34 @@ Print a summary of all target-specific options on the standard output and exit.
 @kindex -Map=@var{mapfile}
 @item -Map=@var{mapfile}
 Print a link map to the file @var{mapfile}.  See the description of the
-@option{-M} option, above.  Specifying a directory as @var{mapfile}
-causes the linker map to be written into a file inside the directory.
-The name of the file is based upon the @var{output} filename with
-@code{.map} appended.
+@option{-M} option, above.  If @var{mapfile} is just the character
+@code{-} then the map will be written to stdout.
+
+Specifying a directory as @var{mapfile} causes the linker map to be
+written as a file inside the directory.  Normally name of the file
+inside the directory is computed as the basename of the @var{output}
+file with @code{.map} appended.   If however the special character
+@code{%} is used then this will be replaced by the full path of the
+output file.  Additionally if there are any characters after the
+@var{%} symbol then @code{.map} will no longer be appended.
+
+@smallexample
+ -o foo.exe -Map=bar                  [Creates ./bar]
+ -o ../dir/foo.exe -Map=bar           [Creates ./bar]
+ -o foo.exe -Map=../dir               [Creates ../dir/foo.exe.map]
+ -o ../dir2/foo.exe -Map=../dir       [Creates ../dir/foo.exe.map]
+ -o foo.exe -Map=%                    [Creates ./foo.exe.map]
+ -o ../dir/foo.exe -Map=%             [Creates ../dir/foo.exe.map]
+ -o foo.exe -Map=%.bar                [Creates ./foo.exe.bar]
+ -o ../dir/foo.exe -Map=%.bar         [Creates ../dir/foo.exe.bar]
+ -o ../dir2/foo.exe -Map=../dir/%     [Creates ../dir/../dir2/foo.exe.map]
+ -o ../dir2/foo.exe -Map=../dir/%.bar [Creates ../dir/../dir2/foo.exe.bar]
+@end smallexample
+
+It is an error to specify more than one @code{%} character.
+
+If the map file already exists then it will be overwritten by this
+operation.
 
 @cindex memory usage
 @kindex --no-keep-memory
diff --git a/ld/lexsup.c b/ld/lexsup.c
index eae64932df..45c29d82ff 100644
--- a/ld/lexsup.c
+++ b/ld/lexsup.c
@@ -22,8 +22,10 @@
 #include "bfd.h"
 #include "bfdver.h"
 #include "libiberty.h"
+#include "filenames.h"
 #include <stdio.h>
 #include <string.h>
+#include <errno.h>
 #include "safe-ctype.h"
 #include "getopt.h"
 #include "bfdlink.h"
@@ -1700,35 +1702,84 @@ parse_args (unsigned argc, char **argv)
   /* Run a couple of checks on the map filename.  */
   if (config.map_filename)
     {
+      char * new_name = NULL;
+      char * percent;
+      int    res = 0;
+
       if (config.map_filename[0] == 0)
 	{
 	  einfo (_("%P: no file/directory name provided for map output; ignored\n"));
 	  config.map_filename = NULL;
 	}
+      else if (strcmp (config.map_filename, "-") == 0)
+	; /* Write to stdout.  Handled in main().  */
+      else if ((percent = strchr (config.map_filename, '%')) != NULL)
+	{
+	  /* FIXME: Check for a second % character and issue an error ?  */
+
+	  /* Construct a map file by replacing the % character with the (full)
+	     output filename.  If the % character was the last character in
+	     the original map filename then add a .map extension.  */
+	  if (percent == config.map_filename)
+	    {
+	      if (percent[1] == 0)
+		res = asprintf (&new_name, "%s.map", output_filename);
+	      else
+		res = asprintf (&new_name, "%s%s",
+				output_filename, percent + 1);
+	    }
+	  else
+	    {
+	      percent[0] = 0;
+	      if (percent[1] == 0)
+		res = asprintf (&new_name, "%s%s.map",
+				config.map_filename, output_filename);
+	      else
+		res = asprintf (&new_name, "%s%s%s",
+				config.map_filename, output_filename,
+				percent + 1);
+	    }
+
+	  /* FIXME: Should we ensure that any directory components in new_name exist ?  */
+	}
       else
 	{
 	  struct stat s;
 
 	  /* If the map filename is actually a directory then create
 	     a file inside it, based upon the output filename.  */
-	  if (stat (config.map_filename, &s) >= 0
-	      && S_ISDIR (s.st_mode))
+	  if (stat (config.map_filename, &s) < 0)
 	    {
-	      char * new_name;
-
-	      /* FIXME: This is a (trivial) memory leak.  */
-	      if (asprintf (&new_name, "%s/%s.map",
-			    config.map_filename, output_filename) < 0)
-		{
-		  /* If this alloc fails then something is probably very
-		     wrong.  Better to halt now rather than continue on
-		     into more problems.  */
-		  einfo (_("%P%F: cannot create name for linker map file: %E\n"));
-		  new_name = NULL;
-		}
-
-	      config.map_filename = new_name;
+	      if (errno != ENOENT)
+		einfo (_("%P: cannot stat linker map file: %E\n"));
+	    }
+	  else if (S_ISDIR (s.st_mode))
+	    {
+	      char lastc = config.map_filename[strlen (config.map_filename) - 1];
+	      res = asprintf (&new_name, "%s%s%s.map",
+			      config.map_filename,
+			      IS_DIR_SEPARATOR (lastc) ? "" : "/",
+			      lbasename (output_filename));
+	    }
+	  else if (! S_ISREG (s.st_mode))
+	    {
+	      einfo (_("%P: linker map file is not a regular file\n"));
+	      config.map_filename = NULL;
 	    }
+	  /* else FIXME: Check write permission ?  */
+	}
+
+      if (res < 0)
+	{
+	  /* If the asprintf failed then something is probably very
+	     wrong.  Better to halt now rather than continue on
+	     into more problems.  */
+	  einfo (_("%P%F: cannot create name for linker map file: %E\n"));
+	}
+      else if (new_name != NULL)
+	{
+	  /* This is a trivial memory leak.  */
+	  config.map_filename = new_name;
 	}
     }
 
diff --git a/ld/testsuite/ld-scripts/map-address.exp b/ld/testsuite/ld-scripts/map-address.exp
index 1f9457a8cd..e8d3683c01 100644
--- a/ld/testsuite/ld-scripts/map-address.exp
+++ b/ld/testsuite/ld-scripts/map-address.exp
@@ -46,6 +46,7 @@ if {[regexp_diff \
     pass $testname
 }
 
+
 set testname "map to directory"
 
 if {![ld_link $ld tmpdir/map-address \
@@ -67,3 +68,49 @@ if {[regexp_diff \
 } else {
     pass $testname
 }
+
+
+set testname "map to % directory"
+
+if {![ld_link $ld tmpdir/map-address \
+	 "$LDFLAGS -T $srcdir/$subdir/map-address.t \
+	  tmpdir/map-address.o \
+	  -Map=tmpdir/% --output fred"]} {
+    fail $testname
+    return
+}
+
+if [is_remote host] then {
+    remote_upload host "tmpdir/fred.map"
+}
+
+if {[regexp_diff \
+	       "tmpdir/fred.map" \
+	       "$srcdir/$subdir/map-address.d"]} {
+    fail $testname
+} else {
+    pass $testname
+}
+
+
+set testname "map to %.foo directory"
+
+if {![ld_link $ld tmpdir/map-address \
+	 "$LDFLAGS -T $srcdir/$subdir/map-address.t \
+	  tmpdir/map-address.o \
+	  -Map=tmpdir/%.foo --output fred"]} {
+    fail $testname
+    return
+}
+
+if [is_remote host] then {
+    remote_upload host "tmpdir/fred.foo"
+}
+
+if {[regexp_diff \
+	       "tmpdir/fred.foo" \
+	       "$srcdir/$subdir/map-address.d"]} {
+    fail $testname
+} else {
+    pass $testname
+}