71064 - Stream classes should have private, unimplemented copy ctor and assignment operators

Status: RESOLVED FIXED

(Core :: XPCOM, defect, P3)

Description

[Simon Fraser [no longer active]]

We've had a number of bug recently (in bookmarks, and cookies) where stream 
assignments have caused bad things to happen, because of premature file closing. 
This happened because streams were being passed by value, not by reference, in a 
few places.

I think we should disallow implicit stream assignments by declaring private copy 
constructors and operator= on the various stream classes.

Comment 1

[Navin Gupta]

Does this bug block #69862, or in other words, prevents #69862 to be landed again.

Comment 2

[Simon Fraser [no longer active]]

No. Fixing this bug would make it a compile-time error if someone passes a stream 
by value, rather than by reference. This means that you can be confident that no 
streams are erroneously copied, leading to the problems we saw before.

Comment 3

[Simon Fraser [no longer active]]

Cc some strong C++ folks

Comment 4

[Simon Fraser [no longer active]]

Attachment: Patch to declare private copy ctors and assignment operators for stream classes

Comment 5

[Simon Fraser [no longer active]]

With this patch, we would have caught pass-by-values of streams in bookmarks and 
cookies at compile time. I've built mozilla with these changes, and the fixes for 
cookie and bookmarks. I have not yet build a commercial tree.

Comment 6

[Navin Gupta]

Good work, you can reassign it to yourself, if you want.

Comment 7

[Simon Fraser [no longer active]]

r/sr anyone?

Comment 8

[Chris Waterson]

r=waterson. do you mean to check in the plevent & nsPipe2 instrumentation stuff?

Comment 9

[Scott Collins]

sr=scc

Comment 10

[Simon Fraser [no longer active]]

Er, whoops. Overzealous patch. Heed only the changes in nsFileStream.h

Comment 11

[rubydoo123]

setting to moz0.9

Comment 12

[Patrick C. Beard]

Shouldn't these classes be retired, in favor of the implementations in Necko?

Comment 13

[Simon Fraser [no longer active]]

They should, but there is way too much code that uses them, and their close ties 
with the venerable nsFileSpec.

Comment 14

[rubydoo123]

moving to mozilla0.9.1

Comment 15

[Simon Fraser [no longer active]]

Fix checked in.