392722 - nsinstall chokes on double slashes in path

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Fabien Tassin]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a7) Gecko/2007081520 GranParadiso/3.0a7
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a7) Gecko/2007081520 GranParadiso/3.0a7

nsinstall is not able to install in paths like /a/b//c/d
if /a/b/ does not already exist. Problem is in mkdirs() which inserts a '\0' preventing 'c' and 'd' to be created.

This has been found while building GranParadiso Alpha7 and trunk on Ubuntu.

Here is what failed:
nsinstall -D /src/firefox-granparadiso-3.0~alpha7/debian/tmp//usr/lib/firefox-granparadiso

nothing after debian/tmp is created.

Reproducible: Always

Steps to Reproduce:
1.
2.
3.



patch follows

Comment 1

[Fabien Tassin]

Attachment: fix multiple slashes bug

patching both mozilla/nsprpub/config/nsinstall.c and mozilla/config/nsinstall.c

Comment 2

[Alexander Sack]

Confirmed ... the patch looks good.

Comment 3

[Fabien Tassin]

Attachment: fix multiple slashes bug, rev2

Add fix for a 3rd file (security/coreconf/nsinstall/nsinstall.c) causing FTBS in xulrunner on Ubuntu when using cdbs.
Fix an issue with install -D foo (no slash at all) introduced by previous patch.

Comment 4

[Benjamin Smedberg]

Comment on attachment 278285 [details] [diff] [review]
fix multiple slashes bug, rev2

r=me for the config/nsinstall.c version, which I will land. The NSPR and NSS build systems are independent, and you'll need to get wtc/nelson to review and land those.

Comment 5

[Benjamin Smedberg]

Comment on attachment 278285 [details] [diff] [review]
fix multiple slashes bug, rev2

r=me for the config/nsinstall.c version, which I will land. The NSPR and NSS build systems are independent, and you'll need to get wtc/nelson to review and land those.

Comment 6

[Benjamin Smedberg]

Checked in config/nsinstall.c Leaving this bug open for the NSPR and NSS versions.

Comment 7

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 278285 [details] [diff] [review]
fix multiple slashes bug, rev2

requesting wtc's review for the nspr portion of the patch.

Comment 8

[Nelson Bolyard (seldom reads bugmail)]

Checked in on trunk for coreconf (NSS)
Checking in nsinstall.c; new revision: 1.9; previous revision: 1.8

Notice: this fix will affect all platforms except Windows and OS/2.
(Does anyone know where the source for the Windows+OS/2 version is?)

Comment 9

[Wan-Teh Chang]

Comment on attachment 278285 [details] [diff] [review]
fix multiple slashes bug, rev2

r=wtc.  Thanks for the patch.

Please change the first line of the file to:

/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
                            ^^^                                       ^^^

The for loop can be rewritten as:

    if ((cp = strrchr(path, '/')))
        while (cp != path && cp[-1] == '/')
            cp--;

Case 1: /a/b//c/d

mkdirs("/a/b//c/d", mode)
  mkdirs("/a/b//c", mode)
    mkdirs("/a/b", mode)
      mkdirs("/a", mode)
        mkdir("/a", mode)
      mkdir("/a/b", mode)
    mkdir("/a/b//c", mode)
  mkdir("/a/b//c/d", mode)

Case 2: /a/b/c/  (trailing /)

mkdirs("/a/b/c/", mode)
  mkdirs("/a/b/c", mode)
    mkdirs("/a/b", mode)
      mkdirs("/a", mode)
        mkdir("/a", mode)
      mkdir("/a/b", mode)
    mkdir("/a/b/c", mode)
  mkdir("/a/b/c/", mode)  <== EEXIST

Fortunately we ignore EEXIST, originally for a different
reason though.

Comment 10

[Wan-Teh Chang]

I checked in the patch on the NSPR trunk (NSPR 4.7).

I made minor changes to the for loop:

    for (cp = strrchr(path, '/'); cp && cp != path && cp[-1] == '/'; cp--)
        ;

Checking in nsinstall.c;
/cvsroot/mozilla/nsprpub/config/nsinstall.c,v  <--  nsinstall.c
new revision: 3.22; previous revision: 3.21
done