avoid '//' prefixes when sysroot is set to '/'

On 25.01.2012 17:45, Joseph S. Myers wrote:
> On Wed, 25 Jan 2012, Matthias Klose wrote:
>
>> This can end up in generation for dependency files, and other files parsing
>> the output. The solution I came up with is to check for sysroot set to '/' and
>> special case this in two places. Afaics, there are no other places.
>
> I could imagine a sysroot path that isn't just '/' but ends with '/'
> resulting in duplicate '/' in the middle of the path - although that's not
> a correctness issue in the way that '//' at the start could be, maybe the
> best check is actually for '/' at end of sysroot (in which case skip the
> '/' at the start of the path within the sysroot)?

as in the attached trailing.diff? built and regression tested.

> Or remove such a '/' at
> end of sysroot at configure time rather than checking for it later....

that's not enough. you can pass --sysroot=/ to the driver as well. I tried that 
in the attached tsr.diff, but I don't like it because the driver then pases 
`-isysroot ""' to the cc*1 binaries, which looks a bit misleading.

   Matthias

Index: gcc/gcc.c
===================================================================
--- gcc/gcc.c	(revision 183514)
+++ gcc/gcc.c	(working copy)
@@ -3417,6 +3417,12 @@
 
     case OPT__sysroot_:
       target_system_root = arg;
+      {
+        int target_system_root_len = strlen (target_system_root);
+
+        if (target_system_root[target_system_root_len - 1] == DIR_SEPARATOR)
+          target_system_root[target_system_root_len - 1] = '\0';
+      }
       target_system_root_changed = 1;
       do_save = false;
       break;
Index: gcc/configure.ac
===================================================================
--- gcc/configure.ac	(revision 183514)
+++ gcc/configure.ac	(working copy)
@@ -767,7 +767,7 @@
 [
  case ${with_sysroot} in
  yes) TARGET_SYSTEM_ROOT='${exec_prefix}/${target_noncanonical}/sys-root' ;;
- *) TARGET_SYSTEM_ROOT=$with_sysroot ;;
+ *) TARGET_SYSTEM_ROOT=`echo $with_sysroot | sed 's,/$,,'` ;;
  esac
    
  TARGET_SYSTEM_ROOT_DEFINE='-DTARGET_SYSTEM_ROOT=\"$(TARGET_SYSTEM_ROOT)\"'
@@ -1830,7 +1830,7 @@
 		*)
 			;;
 	esac
-elif test "x$TARGET_SYSTEM_ROOT" != x; then
+elif test "x$TARGET_SYSTEM_ROOT_DEFINE" != x; then
         SYSTEM_HEADER_DIR=$build_system_header_dir 
 fi
 
@@ -4547,7 +4547,7 @@
 [Define to PREFIX/include if cpp should also search that directory.])
 fi
 
-if test x$host != x$target || test "x$TARGET_SYSTEM_ROOT" != x; then
+if test x$host != x$target || test "x$TARGET_SYSTEM_ROOT_DEFINE" != x; then
   if test "x$with_headers" != x; then
     target_header_dir=$with_headers
   elif test "x$with_sysroot" = x; then

On Thu, 26 Jan 2012, Matthias Klose wrote:

> On 25.01.2012 17:45, Joseph S. Myers wrote:
> > On Wed, 25 Jan 2012, Matthias Klose wrote:
> > 
> > > This can end up in generation for dependency files, and other files
> > > parsing
> > > the output. The solution I came up with is to check for sysroot set to '/'
> > > and
> > > special case this in two places. Afaics, there are no other places.
> > 
> > I could imagine a sysroot path that isn't just '/' but ends with '/'
> > resulting in duplicate '/' in the middle of the path - although that's not
> > a correctness issue in the way that '//' at the start could be, maybe the
> > best check is actually for '/' at end of sysroot (in which case skip the
> > '/' at the start of the path within the sysroot)?
> 
> as in the attached trailing.diff? built and regression tested.

Yes, that's OK (with copyright date updates in incpath.c).