You are viewing a plain text version of this content. The canonical link for it is here.
Posted to scm-dev@maven.apache.org by Brett Porter <br...@apache.org> on 2006/04/27 04:37:56 UTC
Re: svn commit: r397253 - /maven/scm/trunk/maven-scm-api/src/main/java/org/apache/maven/scm/ScmFileSet.java
Doesn't that seem a bit leaky to you? Why is an external entity
modifying the files list directly?
carlos@apache.org wrote:
> Author: carlos
> Date: Wed Apr 26 10:48:41 2006
> New Revision: 397253
>
> URL: http://svn.apache.org/viewcvs?rev=397253&view=rev
> Log:
> Prevent problems when returning an unmodifiable list
>
> Modified:
> maven/scm/trunk/maven-scm-api/src/main/java/org/apache/maven/scm/ScmFileSet.java
>
> Modified: maven/scm/trunk/maven-scm-api/src/main/java/org/apache/maven/scm/ScmFileSet.java
> URL: http://svn.apache.org/viewcvs/maven/scm/trunk/maven-scm-api/src/main/java/org/apache/maven/scm/ScmFileSet.java?rev=397253&r1=397252&r2=397253&view=diff
> ==============================================================================
> --- maven/scm/trunk/maven-scm-api/src/main/java/org/apache/maven/scm/ScmFileSet.java (original)
> +++ maven/scm/trunk/maven-scm-api/src/main/java/org/apache/maven/scm/ScmFileSet.java Wed Apr 26 10:48:41 2006
> @@ -18,8 +18,8 @@
>
> import java.io.File;
> import java.io.IOException;
> +import java.util.ArrayList;
> import java.util.Arrays;
> -import java.util.Collections;
> import java.util.List;
>
> import org.codehaus.plexus.util.FileUtils;
> @@ -40,9 +40,7 @@
> /**
> * List of File objects, all relative to the basedir.
> */
> - private List files = Collections.EMPTY_LIST;
> -
> - private static final File[] EMPTY_FILE_ARRAY = new File[0];
> + private List files = new ArrayList( 0 );
>
> /**
> * Create a file set with no files, only the base directory.
> @@ -51,7 +49,7 @@
> */
> public ScmFileSet( File basedir )
> {
> - this( basedir, EMPTY_FILE_ARRAY );
> + this( basedir, new ArrayList( 0 ) );
> }
>
> /**
>
>
Re: svn commit: r397253 - /maven/scm/trunk/maven-scm-api/src/main/java/org/apache/maven/scm/ScmFileSet.java
Posted by Carlos Sanchez <ca...@apache.org>.
nope
On 4/26/06, Brett Porter <br...@apache.org> wrote:
> Carlos Sanchez wrote:
> > now the behaviour is consistent between empty lists and non empty
> > if we want to make the list unmodifiable, both should but not just the
> > empty one as before
>
> Yes, I agree both should be unmodifiable (I have intellij set up to
> complain at me now when I leak that though I don't always worry about it).
>
> All I meant, was there a specific example where this happened that led
> to the change?
>
> - Brett
>
--
I could give you my word as a Spaniard.
No good. I've known too many Spaniards.
-- The Princess Bride
Re: svn commit: r397253 - /maven/scm/trunk/maven-scm-api/src/main/java/org/apache/maven/scm/ScmFileSet.java
Posted by Brett Porter <br...@apache.org>.
Carlos Sanchez wrote:
> now the behaviour is consistent between empty lists and non empty
> if we want to make the list unmodifiable, both should but not just the
> empty one as before
Yes, I agree both should be unmodifiable (I have intellij set up to
complain at me now when I leak that though I don't always worry about it).
All I meant, was there a specific example where this happened that led
to the change?
- Brett
Re: svn commit: r397253 - /maven/scm/trunk/maven-scm-api/src/main/java/org/apache/maven/scm/ScmFileSet.java
Posted by Carlos Sanchez <ca...@apache.org>.
now the behaviour is consistent between empty lists and non empty
if we want to make the list unmodifiable, both should but not just the
empty one as before
On 4/26/06, Brett Porter <br...@apache.org> wrote:
> Doesn't that seem a bit leaky to you? Why is an external entity
> modifying the files list directly?
>
>
> carlos@apache.org wrote:
> > Author: carlos
> > Date: Wed Apr 26 10:48:41 2006
> > New Revision: 397253
> >
> > URL: http://svn.apache.org/viewcvs?rev=397253&view=rev
> > Log:
> > Prevent problems when returning an unmodifiable list
> >
> > Modified:
> > maven/scm/trunk/maven-scm-api/src/main/java/org/apache/maven/scm/ScmFileSet.java
> >
> > Modified: maven/scm/trunk/maven-scm-api/src/main/java/org/apache/maven/scm/ScmFileSet.java
> > URL: http://svn.apache.org/viewcvs/maven/scm/trunk/maven-scm-api/src/main/java/org/apache/maven/scm/ScmFileSet.java?rev=397253&r1=397252&r2=397253&view=diff
> > ==============================================================================
> > --- maven/scm/trunk/maven-scm-api/src/main/java/org/apache/maven/scm/ScmFileSet.java (original)
> > +++ maven/scm/trunk/maven-scm-api/src/main/java/org/apache/maven/scm/ScmFileSet.java Wed Apr 26 10:48:41 2006
> > @@ -18,8 +18,8 @@
> >
> > import java.io.File;
> > import java.io.IOException;
> > +import java.util.ArrayList;
> > import java.util.Arrays;
> > -import java.util.Collections;
> > import java.util.List;
> >
> > import org.codehaus.plexus.util.FileUtils;
> > @@ -40,9 +40,7 @@
> > /**
> > * List of File objects, all relative to the basedir.
> > */
> > - private List files = Collections.EMPTY_LIST;
> > -
> > - private static final File[] EMPTY_FILE_ARRAY = new File[0];
> > + private List files = new ArrayList( 0 );
> >
> > /**
> > * Create a file set with no files, only the base directory.
> > @@ -51,7 +49,7 @@
> > */
> > public ScmFileSet( File basedir )
> > {
> > - this( basedir, EMPTY_FILE_ARRAY );
> > + this( basedir, new ArrayList( 0 ) );
> > }
> >
> > /**
> >
> >
>
--
I could give you my word as a Spaniard.
No good. I've known too many Spaniards.
-- The Princess Bride