Mailing-List: contact dev-help@directory.apache.org; run by ezmlm
Precedence: bulk
Reply-To: "Apache Directory Developers List" <dev@directory.apache.org>
Received-SPF: pass (athena.apache.org: domain of elecharny@gmail.com
 designates 209.85.134.189 as permitted sender)
DomainKey-Signature: a=rsa-sha1; c=nofws;
        d=gmail.com; s=beta;
        h=received:message-id:date:from:user-agent:mime-version:to:subject:references:in-reply-to:content-type:content-transfer-encoding;
        b=UA/Mj2Qy4jRDlifG9W9Dtww4xAYRkd4k8MB+o4UUEBYvRV3Ot2iqQqQgjN7GQ4uVvazBtl5+tSE/Z7Lxk6p8XFCBggLqdEiPDlRevkZjRcKr7Yu5kv2chMMnSSev2xV1XU6pFkB9L/LkVt3Zyld7qw4nt1JQX2qFJDMRNEzu5BY=
Message-ID: <473B6C07.6080205@gmail.com>
Date: Wed, 14 Nov 2007 22:43:35 +0100
From: Emmanuel Lecharny <elecharny@gmail.com>
User-Agent: Thunderbird 1.5.0.14pre (X11/20071023)
MIME-Version: 1.0
To: dev@directory.apache.org
Subject: 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/
References: <20071114213425.C355E1A9832@eris.apache.org>
In-Reply-To: <20071114213425.C355E1A9832@eris.apache.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit

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