92569 - nsLocalFileUnix needs general clean up. [FIX] {PATCH]

Status: RESOLVED FIXED

(Core :: XPCOM, defect, P3)

Description

[Pete Collins (MDG)]

Clean up and fix uses like mPath.Adopt(foo) to check for 
NS_ERROR_OUT_OF_MEMORY
unchecked accessors like:

newParent->IsDirectory(&targetIsDirectory);

overall style congruity etc . . .

--pete

Comment 1

[Pete Collins (MDG)]

Attachment: a first pass

Comment 2

[Pete Collins (MDG)]

Attachment: updated patch, i think this covers it

Comment 3

[Pete Collins (MDG)]

pushing back

Comment 4

[Pete Collins (MDG)]

I am working on this bug now and am writing some notes for myself to state the
changes being made.

1. removed all the unnecessary (const char*) casting of XPIDLCStrings and using
.get() instead.
2. add proper include files so FreeBSD can statfs instead of returning "not
implemented"
3. some very minor style clean ups. I want to change the tabstops to 2 spaces
instead of 4 but perhaps    i can do that on a second pass.
4. in ::CopyTo, i cloned the param newParent so the method will operate on the
clone instead of the callers param. I beleive win and mac implement this way as
well.

--pete

Comment 5

[Pete Collins (MDG)]

Attachment: general cleanup

Comment 6

[Pete Collins (MDG)]

Attachment: patch for review

need review

Comment 7

[Pete Collins (MDG)]

Attachment: for review

Comment 8

[Brendan Eich [:brendan]]

Adding other nsLocalFileUnix buddies.

/be

Comment 9

[Brendan Eich [:brendan]]

Comment on attachment 59085 [details] [diff] [review]
for review

Need to indent the return rv; below:

@@ -564,7 +578,8 @@
	 } else {
	     // make sure that the target is actually a directory
	     PRBool targetIsDirectory;
-	     newParent->IsDirectory(&targetIsDirectory);
+	     if (NS_FAILED(rv = newParent->IsDirectory(&targetIsDirectory)))
+	     return rv;

Otherwise looks good to me -- r/sr=[e-mail], whichever ya need.

/be

Comment 10

[Brendan Eich [:brendan]]

Someone else should give the patch a good r=, I was in a hurry for lunch (but
I'm not worried).

/be

Comment 11

[Pete Collins (MDG)]

Attachment: fixed indentation that Brendan pointed out

Comment 12

[Doug Turner (:dougt)]

Comment on attachment 59085 [details] [diff] [review]
for review

what brendan said.

Comment 13

[Pete Collins (MDG)]

Checked in.

I am going to leave this open and do a style modification and change the tabstop
to 2.


--pete

Comment 14

[Brendan Eich [:brendan]]

Pete: why change the indentation?  A bunch of xpcom files (in particular,
xpcom/io files) use four space.  I prefer it, and so do others with vested
interests here.  You should not change it unilaterally, even though you are
doing great work and touching more lines than the rest of us vested interested
parties, I think.  What's your motivation?

/be

Comment 15

[Pete Collins (MDG)]

Attachment: w/ 2 space indentation and style cleanup

Comment 16

[Pete Collins (MDG)]

Brendan, i cleande up the comments and white space etc. 
My motivation is to just make the file more uniform and easier to read.

I prefer 2 space indentation but i can put it back to 4 if you like.
Two makes it easier to read IMHO.

Anyway, i'd like to land this now.

I can keep it at 4 and just land the pure style and white space cleanup here.
I won't need another review. No code has been altered.

Thanks

--pete

Comment 17

[Pete Collins (MDG)]

Attachment: purely a style/white space/comment cleanup patch

Comment 18

[Pete Collins (MDG)]

Indentation reamins at 4 spaces.

I will check it in, in about a half an hour.

Thanks

--pete

Comment 19

[Brendan Eich [:brendan]]

Comment on attachment 59563 [details] [diff] [review]
purely a style/white space/comment cleanup patch 

Pete, how about a diff -wu?  Also, please don't make the license comment a
doc-comment -- don't mess with anything in the "license block", in general.

/be

Comment 20

[Pete Collins (MDG)]

Thanks Brendan. 

Part two now checked in.

--pete