ant-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Martin Gainty <mgai...@hotmail.com>
Subject Re: Mapped resources NPEs - Potential fix causes a failure in one specific test
Date Fri, 09 Feb 2018 13:00:12 GMT
How many times have we seen [] implementation throw StackOverflow?
this is because you are inserting elements into an already overloaded stack
what happens if your stack cannot accomodate the extra element..StackOverflow

refactoring to an implementation of Collection:

ArrayList<T> list=null;


//now i can test the list for null
if(list == null)

and if you absolutely positively need an array
T []someArray = list.toArray()

+1 for Jaikiran patch

Martin
______________________________________________



________________________________
From: Jaikiran Pai <jai.forums2013@gmail.com>
Sent: Friday, February 9, 2018 5:05 AM
To: dev@ant.apache.org
Subject: Re: Mapped resources NPEs - Potential fix causes a failure in one specific test


On 09/02/18 2:47 PM, Dominique Devienne wrote:
> On Fri, Feb 9, 2018 at 10:06 AM, Jaikiran Pai <jai.forums2013@gmail.com>
> wrote:
>
>> I need some inputs on how we should go about this specific change/test?
>> Should this test continue to expect a exception or is it fine to expect
>> that target to complete cleanly (without copying anything)?
>>
> What's the source of the NPE?
> If it's an implementation detail, then it's fine to no longer throw IMHO.
We have FileNameMapper interface which has a:

String[] mapFileName(String sourceFileName);

API. The contract/javadoc of this API says:

      * Returns an array containing the target filename(s) for the
      * given source file.
      *
      * <p>if the given rule doesn't apply to the source file,
      * implementation must return null....

We have an implementation of this interface, the IdentityMapper, whose
implementation so far, IMO, wasn't following this contract. If it was
passed a null source file name, that implementation would return an
String[] with one element and the contained element in that array would
be null.

Callers of this API are expected to understand that the API itself might
return null, so they used to just check the return value and not its
contents. They would then go and start working on these individual
elements in the array and start running into NPEs in a bunch of places.

The change I made was to the implementation of IdentityMapper, so that
it returns null instead of an array with one null element, when the
input to it is null. That way, the callers' logic of checking the return
value for null, is enough for them to skip such resources.

So to me, this does look like something that we should take care off in
that specific test, so that it no longer expects a build exception to be
thrown.

-Jaikiran


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message