Mailing-List: contact dev-help@harmony.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@harmony.apache.org
Received-SPF: pass (herse.apache.org: domain of sianjanuary@googlemail.com
 designates 66.249.92.174 as permitted sender)
DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws;
        s=beta; d=googlemail.com;
        h=received:message-id:date:from:to:subject:in-reply-to:mime-version:content-type:references;
        b=Gfam0XF4IdPbpJj5Nf5P6Hld9iGsJ+Y5cvW8EMa38Qwf6gjkWFaENVRAXOky4a1aBfn3Ru4EnNEhXtARfh5rweLrzO3MpwWwkAkJJl3uFEJiajkDBb39uXmfb5DtDWFoLf76hKakOcFGn5uuSN0gLftfTWSu7t6yAn2PoyXFI50=
Message-ID: <fc2fc95c0612200851h5a78888l7c469e8ddfbaf915@mail.gmail.com>
Date: Wed, 20 Dec 2006 16:51:23 +0000
From: "Sian January" <sianjanuary@googlemail.com>
To: dev@harmony.apache.org, geir@pobox.com
Subject: Re: [classlib] FindBugs results for luni (HARMONY-2811)
In-Reply-To: <458963CD.1020803@pobox.com>
MIME-Version: 1.0
Content-Type: multipart/alternative;
	boundary="----=_Part_25916_18203170.1166633483878"
References: <c3755b3a0612200805r4798c3f5u9eecaeb966b8ac7e@mail.gmail.com>
	 <458963CD.1020803@pobox.com>

------=_Part_25916_18203170.1166633483878
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

Hi Geir,

The patch is mostly 1-line changes to different source files, which is why
it's so big.  It's not actually 47K of new code.  Also about half of it is
the same change to all the Locale classes.  However even without the Locale
changes the patch covers about 30 classes so I thought it might be easier
for people to look at one patch rather than 30.  However if you would still
like me to break it down I will try and see if I can break it down by
package or by type of bug or something like that.

Regards,

Sian


On 20/12/06, Geir Magnusson Jr. <geir@pobox.com> wrote:
>
> 47K?
>
> Can we have the patch in digestible pieces?
>
> geir
>
>
> Alexey Petrenko wrote:
> > Guys,
> >
> > Sian January made a huge job of fixing FindBugs warnings for luni
> > module. The resulting patch size is 47K!
> > You can find the patch attached to HARMONY-2811 [1].
> >
> > I agree with the most of changes with small exceptions and need luni
> > experts advise.
> >
> > Index:
> > luni/src/main/java/org/apache/harmony/luni/platform/AdapterManager.java
> > ============================================================@@ -35,7
> > +35,7 @@
> >  private final HashMap<Class, List<IAdapterFactory>> factories = new
> > HashMap<Class, List<IAdapterFactory>>();
> >
> >  public Object getAdapter(IAdaptable adaptable, Class adapterType) {
> > - List factoryList = factories.get(adaptable);
> > + List factoryList = factories.get(adapterType);
> >  if (factoryList != null) {
> >  for (Iterator factoryItr = factoryList.iterator(); factoryItr
> >  .hasNext();) {
> > ============================================================
> > FindBugs reports here that the code passes the parameter with the
> > wrong type to get method.
> > Can someone say is this substitution OK?
> >
> > Index:
> > luni/src/main/java/org/apache/harmony/luni/platform/DebugMemorySpy.java
> > ===========================================================
> > ---
> luni/src/main/java/org/apache/harmony/luni/platform/DebugMemorySpy.java
> > (revision 486101)
> > +++
> luni/src/main/java/org/apache/harmony/luni/platform/DebugMemorySpy.java
> > (working copy)
> > @@ -25,7 +25,7 @@
> >  */
> > final class DebugMemorySpy extends AbstractMemorySpy {
> >
> > - private final boolean stackDump = true;
> > + private static final boolean stackDump = true;
> > ============================================================FindBugs
> > reports that stackDump is not used. I suggest to simply remove it.
> >
> > There are also few array clonings like the following:
> > @@ -485,7 +485,7 @@
> >      * @return java.net.URL[]
> >      */
> >     public URL[] getURLs() {
> > -        return orgUrls;
> > +        return orgUrls.clone();
> >     }
> > This will definitely increase the code stability but could cause
> > performance degradation.
> > Should we accept or decline all such cases or should discuss them one by
> > one.
> > I would accept such changes.
> >
> > Thoughts?
> >
> > BTW the patch passes all the unit tests.
> >
> > SY, Alexey
> >
> > P.S. I'll apply the patch without first mentioned hunk to not to loose
> > the code compatibility with the huge patch.
>


-- 
Sian January

IBM Java Technology Centre, UK

------=_Part_25916_18203170.1166633483878--