You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@directory.apache.org by Emmanuel Lecharny <el...@gmail.com> on 2007/11/14 22:43:35 UTC

Re: svn commit: r595072 - in /directory/shared/branches/bigbang: asn1/src/main/java/org/apache/directory/shared/asn1/der/ ldap/src/main/java/org/apache/directory/shared/ldap/util/

Hi felix,

don't spend too much time on DERxxx classes, they won't last forever ;)

Otherwise, be sure to run mvn -Dintegration test before committing, just 
to be sure nothing is broken (may be you have run those tests, but I 
wanted to double check :)

Thanks for cleaning the code ! This is, IMO, one of the best way to get 
into the code and try to understand it.

felixk@apache.org wrote:
> Author: felixk
> Date: Wed Nov 14 13:34:15 2007
> New Revision: 595072
>
> URL: http://svn.apache.org/viewvc?rev=595072&view=rev
> Log:
> Method may fail to close stream
>
> The method creates an IO stream object, does not assign it to any fields, pass it to other methods that might close it, or return it, and does not appear to close the stream on all paths out of the method.  This may result in a file descriptor leak.  It is generally a good idea to use a finally block to ensure that streams are closed.
>
> Modified:
>     directory/shared/branches/bigbang/asn1/src/main/java/org/apache/directory/shared/asn1/der/ASN1InputStream.java
>     directory/shared/branches/bigbang/asn1/src/main/java/org/apache/directory/shared/asn1/der/DERApplicationSpecific.java
>     directory/shared/branches/bigbang/ldap/src/main/java/org/apache/directory/shared/ldap/util/PropertiesUtils.java
>
> Modified: directory/shared/branches/bigbang/asn1/src/main/java/org/apache/directory/shared/asn1/der/ASN1InputStream.java
> URL: http://svn.apache.org/viewvc/directory/shared/branches/bigbang/asn1/src/main/java/org/apache/directory/shared/asn1/der/ASN1InputStream.java?rev=595072&r1=595071&r2=595072&view=diff
> ==============================================================================
> --- directory/shared/branches/bigbang/asn1/src/main/java/org/apache/directory/shared/asn1/der/ASN1InputStream.java (original)
> +++ directory/shared/branches/bigbang/asn1/src/main/java/org/apache/directory/shared/asn1/der/ASN1InputStream.java Wed Nov 14 13:34:15 2007
> @@ -188,15 +188,22 @@
>                  return new DERNull();
>              case DERObject.SEQUENCE | DERObject.CONSTRUCTED:
>                  ASN1InputStream ais = new ASN1InputStream( bytes );
> -
> +                DEREncodable obj = null;
>                  DERSequence sequence = new DERSequence();
>  
> -                DEREncodable obj = ais.readObject();
> -
> -                while ( obj != null )
> +                try
>                  {
> -                    sequence.add( obj );
>                      obj = ais.readObject();
> +    
> +                    while ( obj != null )
> +                    {
> +                        sequence.add( obj );
> +                        obj = ais.readObject();
> +                    }
> +                }
> +                finally
> +                {
> +                    ais.close();
>                  }
>  
>                  return sequence;
> @@ -204,12 +211,19 @@
>                  ais = new ASN1InputStream( bytes );
>                  DERSet set = new DERSet();
>  
> -                obj = ais.readObject();
> -
> -                while ( obj != null )
> +                try
>                  {
> -                    set.add( obj );
>                      obj = ais.readObject();
> +    
> +                    while ( obj != null )
> +                    {
> +                        set.add( obj );
> +                        obj = ais.readObject();
> +                    }
> +                }
> +                finally
> +                {
> +                    ais.close();
>                  }
>  
>                  return set;
> @@ -292,25 +306,32 @@
>  
>                      ais = new ASN1InputStream( bytes );
>  
> -                    DEREncodable encodable = ais.readObject();
> -
> -                    // Explicitly tagged - if it isn't we'd have to tell from
> -                    // the context.
> -                    if ( ais.available() == 0 )
> +                    try
>                      {
> -                        return new DERTaggedObject( true, tagNo, encodable, bytes );
> -                    }
> -
> -                    // Another implicit object, create a sequence.
> -                    DERSequence derSequence = new DERSequence();
> +                        DEREncodable encodable = ais.readObject();
> +    
> +                        // Explicitly tagged - if it isn't we'd have to tell from
> +                        // the context.
> +                        if ( ais.available() == 0 )
> +                        {
> +                            return new DERTaggedObject( true, tagNo, encodable, bytes );
> +                        }
> +    
> +                        // Another implicit object, create a sequence.
> +                        DERSequence derSequence = new DERSequence();
> +    
> +                        while ( encodable != null )
> +                        {
> +                            derSequence.add( encodable );
> +                            encodable = ais.readObject();
> +                        }
>  
> -                    while ( encodable != null )
> +                        return new DERTaggedObject( false, tagNo, derSequence );
> +                    }
> +                    finally
>                      {
> -                        derSequence.add( encodable );
> -                        encodable = ais.readObject();
> +                        ais.close();
>                      }
> -
> -                    return new DERTaggedObject( false, tagNo, derSequence );
>                  }
>  
>                  return new DERUnknownTag( tag, bytes );
>
> Modified: directory/shared/branches/bigbang/asn1/src/main/java/org/apache/directory/shared/asn1/der/DERApplicationSpecific.java
> URL: http://svn.apache.org/viewvc/directory/shared/branches/bigbang/asn1/src/main/java/org/apache/directory/shared/asn1/der/DERApplicationSpecific.java?rev=595072&r1=595071&r2=595072&view=diff
> ==============================================================================
> --- directory/shared/branches/bigbang/asn1/src/main/java/org/apache/directory/shared/asn1/der/DERApplicationSpecific.java (original)
> +++ directory/shared/branches/bigbang/asn1/src/main/java/org/apache/directory/shared/asn1/der/DERApplicationSpecific.java Wed Nov 14 13:34:15 2007
> @@ -68,8 +68,16 @@
>  
>  
>      public DEREncodable getObject() throws IOException
> -    {
> -        return new ASN1InputStream( getOctets() ).readObject();
> +    {   
> +        final ASN1InputStream ais = new ASN1InputStream( getOctets() );
> +        try
> +        {
> +            return ais.readObject();
> +        }
> +        finally
> +        {
> +            ais.close();
> +        }
>      }
>  
>  
>
> Modified: directory/shared/branches/bigbang/ldap/src/main/java/org/apache/directory/shared/ldap/util/PropertiesUtils.java
> URL: http://svn.apache.org/viewvc/directory/shared/branches/bigbang/ldap/src/main/java/org/apache/directory/shared/ldap/util/PropertiesUtils.java?rev=595072&r1=595071&r2=595072&view=diff
> ==============================================================================
> --- directory/shared/branches/bigbang/ldap/src/main/java/org/apache/directory/shared/ldap/util/PropertiesUtils.java (original)
> +++ directory/shared/branches/bigbang/ldap/src/main/java/org/apache/directory/shared/ldap/util/PropertiesUtils.java Wed Nov 14 13:34:15 2007
> @@ -200,7 +200,15 @@
>          {
>              try
>              {
> -                properties.load( new FileInputStream( file ) );
> +                final FileInputStream fis = new FileInputStream( file );
> +                try
> +                {
> +                    properties.load( fis );
> +                }
> +                finally
> +                {
> +                    fis.close();
> +                }
>              }
>              catch ( IOException e )
>              {
>
>
>
>   


-- 
--
cordialement, regards,
Emmanuel Lécharny
www.iktek.com
directory.apache.org



Re: svn commit: r595072 - in /directory/shared/branches/bigbang: asn1/src/main/java/org/apache/directory/shared/asn1/der/ ldap/src/main/java/org/apache/directory/shared/ldap/util/

Posted by Felix Knecht <fe...@apache.org>.
Pierre-Arnaud Marcelot schrieb:
> Hi Felix,
>
> Try "mvn -Dintegration test". ;)

Puhh... Thanks!
In German we say "Wer lesen kann hat mehr vom Leben" (~ Celui qui peut
lire a plus de la vie, who can read enjoys life better)

Felix
>
> Regards,
> Pierre-Arnaud Marcelot
>
> On Nov 15, 2007 6:26 AM, Felix Knecht <felixk@apache.org
> <ma...@apache.org>> wrote:
>
>     Emmanuel Lecharny schrieb:
>     >>
>     > no, mvn -Dintegration test. But it may be the same.
>
>     It doesn't works :(
>
>
>     felix@donar ~/svn/apache/directory/shared/bigbang $ mvn -versino
>     Maven version: 2.0.7
>     Java version: 1.6.0_03
>     OS name: "linux" version: "2.6.23-gentoo-r1 " arch: "amd64"
>
>     felix@donar ~/svn/apache/directory/shared/bigbang $ mvn -Dintegration
>     [INFO] Scanning for projects...
>     [INFO] Reactor build order:
>     [INFO]   Apache Directory Shared
>     [INFO]   Apache Directory ASN.1 Shared
>     [INFO]   Apache Directory Protocol Ldap Shared Constants
>     [INFO]   Apache Directory Protocol Ldap Shared
>     [INFO]   Apache Directory JNDI Shared
>     [INFO]   Apache Directory MINA ASN.1 Codec Shared
>     [INFO]   Apache Directory Protocol Ldap Converters
>     [INFO]
>     ------------------------------------------------------------------------
>     [ERROR] BUILD FAILURE
>     [INFO]
>     ------------------------------------------------------------------------
>     [INFO] You must specify at least one goal. Try 'install'
>     [INFO]
>     ------------------------------------------------------------------------
>     [INFO] For more information, run Maven with the -e switch
>     [INFO]
>     ------------------------------------------------------------------------
>
>     [INFO] Total time: < 1 second
>     [INFO] Finished at: Thu Nov 15 06:23:19 CET 2007
>     [INFO] Final Memory: 3M/119M
>     [INFO]
>     ------------------------------------------------------------------------
>
>
>     Felix
>
>


Re: svn commit: r595072 - in /directory/shared/branches/bigbang: asn1/src/main/java/org/apache/directory/shared/asn1/der/ ldap/src/main/java/org/apache/directory/shared/ldap/util/

Posted by Pierre-Arnaud Marcelot <pa...@marcelot.net>.
Hi Felix,

Try "mvn -Dintegration test". ;)

Regards,
Pierre-Arnaud Marcelot

On Nov 15, 2007 6:26 AM, Felix Knecht <fe...@apache.org> wrote:

> Emmanuel Lecharny schrieb:
> >>
> > no, mvn -Dintegration test. But it may be the same.
>
> It doesn't works :(
>
>
> felix@donar ~/svn/apache/directory/shared/bigbang $ mvn -versino
> Maven version: 2.0.7
> Java version: 1.6.0_03
> OS name: "linux" version: "2.6.23-gentoo-r1" arch: "amd64"
>
> felix@donar ~/svn/apache/directory/shared/bigbang $ mvn -Dintegration
> [INFO] Scanning for projects...
> [INFO] Reactor build order:
> [INFO]   Apache Directory Shared
> [INFO]   Apache Directory ASN.1 Shared
> [INFO]   Apache Directory Protocol Ldap Shared Constants
> [INFO]   Apache Directory Protocol Ldap Shared
> [INFO]   Apache Directory JNDI Shared
> [INFO]   Apache Directory MINA ASN.1 Codec Shared
> [INFO]   Apache Directory Protocol Ldap Converters
> [INFO]
> ------------------------------------------------------------------------
> [ERROR] BUILD FAILURE
> [INFO]
> ------------------------------------------------------------------------
> [INFO] You must specify at least one goal. Try 'install'
> [INFO]
> ------------------------------------------------------------------------
> [INFO] For more information, run Maven with the -e switch
> [INFO]
> ------------------------------------------------------------------------
> [INFO] Total time: < 1 second
> [INFO] Finished at: Thu Nov 15 06:23:19 CET 2007
> [INFO] Final Memory: 3M/119M
> [INFO]
> ------------------------------------------------------------------------
>
>
> Felix
>
>

Re: svn commit: r595072 - in /directory/shared/branches/bigbang: asn1/src/main/java/org/apache/directory/shared/asn1/der/ ldap/src/main/java/org/apache/directory/shared/ldap/util/

Posted by Felix Knecht <fe...@apache.org>.
Emmanuel Lecharny schrieb:
>>   
> no, mvn -Dintegration test. But it may be the same.

It doesn't works :(


felix@donar ~/svn/apache/directory/shared/bigbang $ mvn -versino
Maven version: 2.0.7
Java version: 1.6.0_03
OS name: "linux" version: "2.6.23-gentoo-r1" arch: "amd64"

felix@donar ~/svn/apache/directory/shared/bigbang $ mvn -Dintegration
[INFO] Scanning for projects...
[INFO] Reactor build order:
[INFO]   Apache Directory Shared
[INFO]   Apache Directory ASN.1 Shared
[INFO]   Apache Directory Protocol Ldap Shared Constants
[INFO]   Apache Directory Protocol Ldap Shared
[INFO]   Apache Directory JNDI Shared
[INFO]   Apache Directory MINA ASN.1 Codec Shared
[INFO]   Apache Directory Protocol Ldap Converters
[INFO] ------------------------------------------------------------------------
[ERROR] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] You must specify at least one goal. Try 'install'
[INFO] ------------------------------------------------------------------------
[INFO] For more information, run Maven with the -e switch
[INFO] ------------------------------------------------------------------------
[INFO] Total time: < 1 second
[INFO] Finished at: Thu Nov 15 06:23:19 CET 2007
[INFO] Final Memory: 3M/119M
[INFO] ------------------------------------------------------------------------


Felix


Re: svn commit: r595072 - in /directory/shared/branches/bigbang: asn1/src/main/java/org/apache/directory/shared/asn1/der/ ldap/src/main/java/org/apache/directory/shared/ldap/util/

Posted by Emmanuel Lecharny <el...@gmail.com>.
Felix Knecht wrote:
>> Otherwise, be sure to run mvn -Dintegration test before committing, just
>> to be sure nothing is broken (may be you have run those tests, but I
>> wanted to double check :)
>>     
>
> I did a mvn clean install, which runs the tests also. I'm not sure if just -Dintegration has some effects. 
It's very different. It runs much more tests than mvn clean install. 
(and it last.... forever ;)
> Did you meant
> -Pintegration (profile integration)?
>   
no, mvn -Dintegration test. But it may be the same.

-- 
--
cordialement, regards,
Emmanuel Lécharny
www.iktek.com
directory.apache.org



Re: svn commit: r595072 - in /directory/shared/branches/bigbang: asn1/src/main/java/org/apache/directory/shared/asn1/der/ ldap/src/main/java/org/apache/directory/shared/ldap/util/

Posted by Felix Knecht <fe...@apache.org>.
Emmanuel Lecharny schrieb:
> Hi felix,
> 
> don't spend too much time on DERxxx classes, they won't last forever ;)

Ok, thanks.

> 
> Otherwise, be sure to run mvn -Dintegration test before committing, just
> to be sure nothing is broken (may be you have run those tests, but I
> wanted to double check :)

I did a mvn clean install, which runs the tests also. I'm not sure if just -Dintegration has some effects. Did you meant
-Pintegration (profile integration)?

Felix

> 
> Thanks for cleaning the code ! This is, IMO, one of the best way to get
> into the code and try to understand it.

My thoughts ;)

Felix