84738 - leafName returns a wrong string [PATCH]

Status: RESOLVED FIXED

(Core :: XPCOM, defect, P3)

Description

[Pete Collins (MDG)]

leafName returns a wrong string after manipulation
from parent.

Output of Test:

js> o;
[xpconnect wrapped nsIFile @ 0x80972c8]
js> o.path;
/tmp
js> o.leafName;
tmp
js> o.path;
/tmp
js> o.parent.path;
/
js> o.leafName;
mp
js> o.path;
//mp
js>

You can assign this to me if you want and i'll take a look at it.

--pete

Comment 1

[Edward Kandrot]

Does not appear to be an XPCOM problem to me from the discription.  Reassigning
as reuqested.  Thanks.

Comment 2

[John Bandhauer]

kandrot: Last I checked the implementations of nsIFile were in xpcom.

Comment 3

[Pete Collins (MDG)]

Attachment: patch

Comment 4

[Pete Collins (MDG)]

OK, This patch fixes the problem

--pete

Comment 5

[Pete Collins (MDG)]

Setting Target to 0.9.3

--pete

Comment 6

[Pete Collins (MDG)]

Test output after fix:


js> o;
[xpconnect wrapped nsIFile @ 0x814b678]
js> o.path;
/tmp
js> o.parent.path;
/
js> o.path;
/tmp
js> o.parent.path;
/
js> r;
[xpconnect wrapped nsIFile @ 0x814bcb0]
js> r.path;
/
js> r.parent.path;
/
js> r.path;
/
js> 

Comment 7

[Doug Turner (:dougt)]

Where is |orig| freed?

nits:

Why did the whitespace change on @@ -929,7 +930,7 @@

Use PRBool instead of bool.

Even though I uses curly braces like you, the heathen owners don't:       

if (slashp == buffer)
-        slashp++;
+    {

Zap that space between the "if" and "else":
+
+    if(root)
+      mPath.Adopt(orig);
+
+    else

Comment 8

[Pete Collins (MDG)]

Attachment: changes per doug's comments

Comment 9

[Pete Collins (MDG)]

Where is |orig| freed?

See path  #39511. . .

nits:

> Why did the whitespace change on @@ -929,7 +930,7 @@

This is a rsult of previous tabs that someone checked in. 

> Use PRBool instead of bool.

Yes, forgot. It in now.

> Even though I uses curly braces like you, the heathen owners don't:       

if (slashp == buffer)
-        slashp++;
+    {


The curly braces are to accomondate the assignment of `root=PR_TRUE'
I don't use curly braces for one liners either . . .  ;-)

> Zap that space between the "if" and "else":

Zapped  . . . 

--pete

Comment 10

[Doug Turner (:dougt)]

nit: 
if thou brace "else", thou must not forsaken "if".  
spacing problem still is there (but I am not one to talk).

Fix that and r=dougt

Comment 11

[Pete Collins (MDG)]

Attachment: fix for 'return NS_ERROR_FAILURE' ws

Comment 12

[Pete Collins (MDG)]

Attachment: unforsaking the if . . .

Comment 13

[Pete Collins (MDG)]

Attachment: minor ws tweak

Comment 14

[Johnny Stenback (:jst)]

Looks good to me, but please change the brace style in the changes to match
what's done in the rest of thise file, i.e.:

    if (...) {
        ..
    }

and not:

    if (...)
    {
        ...
    }

and also use 4-space indentation, just like the other code in that file does.
Consistency with surrounding code overways correctness when it comes to
whitespace useage since there is no correct way! :-)

With that fixed, sr=jst

Comment 15

[Johnny Stenback (:jst)]

s/overways/overweighs/

Comment 16

[Pete Collins (MDG)]

Attachment: patch with jst's suggestions applied

Comment 17

[Pete Collins (MDG)]

I checked this in this morning

resolving fixed

--pete