harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alexey Petrenko" <alexey.a.petre...@gmail.com>
Subject Re: [classlib] FindBugs results for luni (HARMONY-2811)
Date Thu, 21 Dec 2006 09:50:00 GMT
Ok, since nobody objects...
I'll apply the patch almost as is. I agree with Sian on his comment
about AdapterManager modification.
But I'll remove DebugMemorySpy update.

SY, Alexey

2006/12/20, Alexey Petrenko <alexey.a.petrenko@gmail.com>:
> 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.
>

Mime
View raw message