You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by lu...@apache.org on 2012/02/16 17:17:14 UTC

svn commit: r1245061 - /commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/FastMathLiteralArrays.java

Author: luc
Date: Thu Feb 16 16:17:14 2012
New Revision: 1245061

URL: http://svn.apache.org/viewvc?rev=1245061&view=rev
Log:
Removed unneeded clone.

The clone did not protect the array used, only the reference ones.
JIRA: MATH-650

Modified:
    commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/FastMathLiteralArrays.java

Modified: commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/FastMathLiteralArrays.java
URL: http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/FastMathLiteralArrays.java?rev=1245061&r1=1245060&r2=1245061&view=diff
==============================================================================
--- commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/FastMathLiteralArrays.java (original)
+++ commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/FastMathLiteralArrays.java Thu Feb 16 16:17:14 2012
@@ -6139,7 +6139,7 @@ class FastMathLiteralArrays {
      * @return a clone of the data array.
      */
     static double[] loadExpIntA() {
-        return EXP_INT_A.clone();
+        return EXP_INT_A;
     }
     /**
      * Load "EXP_INT_B".
@@ -6147,7 +6147,7 @@ class FastMathLiteralArrays {
      * @return a clone of the data array.
      */
     static double[] loadExpIntB() {
-        return EXP_INT_B.clone();
+        return EXP_INT_B;
     }
     /**
      * Load "EXP_FRAC_A".
@@ -6155,7 +6155,7 @@ class FastMathLiteralArrays {
      * @return a clone of the data array.
      */
     static double[] loadExpFracA() {
-        return EXP_FRAC_A.clone();
+        return EXP_FRAC_A;
     }
     /**
      * Load "EXP_FRAC_B".
@@ -6163,7 +6163,7 @@ class FastMathLiteralArrays {
      * @return a clone of the data array.
      */
     static double[] loadExpFracB() {
-        return EXP_FRAC_B.clone();
+        return EXP_FRAC_B;
     }
     /**
      * Load "LN_MANT".
@@ -6171,6 +6171,6 @@ class FastMathLiteralArrays {
      * @return a clone of the data array.
      */
     static double[][] loadLnMant() {
-        return LN_MANT.clone();
+        return LN_MANT;
     }
 }



Re: svn commit: r1245061 - /commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/FastMathLiteralArrays.java

Posted by Luc Maisonobe <Lu...@free.fr>.
Hi all,

Le 23/02/2012 16:45, Jörg Schaible a écrit :
> Gilles Sadowski wrote:
> 
>>>>>>>>>> URL: http://svn.apache.org/viewvc?rev=1245061&view=rev
>>>>>>>>>> Log:
>>>>>>>>>> Removed unneeded clone.
>>>>>>>>>>
>>>>>>>>>> The clone did not protect the array used, only the reference
>>>>>>>>>> ones. JIRA: MATH-650
>>>>>>>>>
>>>>>>>>> -1
>>>>>>>>>
>>>>>>>>> That was the whole point of the clone - to protect the original
>>>>>>>>> external data.
>>>>>>>>
>>>>>>>> Please (re-)explain what you mean by "protect".  Cf. my comment
>>>>>>>> on the JIRA page.
>>>>>>>
>>>>>>> See also your comment of 30/Nov/11 00:31.
>>>>>>
>>>>>> I know, but my latest comment overrides that one.
>>>>>>
>>>>>>> The arrays in FastMathLiteralArrays are private, but the access
>>>>>>> methods are not, and returning an array allows the caller to modify
>>>>>>> array elements.
>>>>>>
>>>>>> It's true, but unless I'm mistaken, it doesn't matter since in the
>>>>>> end there is one and only one array that will be _used_ (the
>>>>>> modified one) and having a pristine copy somewhere else will not
>>>>>> prevent the dire consequences of using the modified one... ;-/
>>>>>
>>>>> Not sure I follow that.
>>>>>
>>>>> Without clone, the array entries can be changed at any time.
>>>>>
>>>>> With clone, there are two pristine copies; neither can be changed
>>>>> directly as they are stored in private arrays.
>>>>
>>>> The question is: Why calling "clone"?
>>>> The answer is: You get a copy and you cannot change the array stored in
>>>> "FastMathLiteralArrays".
>>>> But that doesn't save you from the bug that consists in changing an
>>>> array entry from within "FastMath".
>>>> If that potential bug exists, some methods will produce erroneous
>>>> values because they use the copy, not the pristine original array
>>>> stored in "FastMathLiteralArrays". [After loading, the original array
>>>> is never accessed again.]
>>>>
>>>> Now if you just obtain a reference to the original array (i.e. no call
>>>> to "clone"), the bug will indeed modify it.
>>>> But the consequences are not worse than in the above scenario: The
>>>> exact same erroneous values will be computed.
>>>
>>> Not strictly true, because it's not only the FastMath class that can
>>> corrupt the array.
>>
>> Only "our" classes (in the package "o.a.c.m.util") can access (and
>> corrupt) the arrays.
>>
>>>
>>> Of course clone does not protect against bugs in FastMath; only array
>>> entry getters can do that.
>>
>> Yes. That's what I said.
>>
>>> But that's not the point;
>>
>> Yes, that's the point.
>>
>>> since the arrays were (at your insistence)
>>> moved out of FastMath itself, this necessarily meant exposing the
>>> contents to some degree.
>>> Using clone closes that potential exposure - as would using array entry
>>> getters.
>>
>> "clone" provides protection against CM developers, which I deem
>> unnecessary, since those same developers would also be able to make the
>> "mistake" of modifying the contents "FastMathLiteralArrays". [And the same
>> mistake could also happen if the arrays were stored inside the "FastMath"
>> class.]
> 
> I support Gilles here, the methods are package scoped and therefore 
> internal. A CM user will have to create an own package with same name (which 
> will not work for sealed jars or with OSGi bundles anyway) or use reflection 
> to access them. At that stage he should already know what he does. For CM 
> itself there's no need to pay the penalty to create a copy.

I don't have any clear cut opinion here, but has said in another thread
in this list we do have a veto on a code change, and this cannot be
ignored as per Apache rules.

So I a  going to undo that change and get the clone back.

Luc


> 
> - Jörg
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
> 


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


Re: svn commit: r1245061 - /commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/FastMathLiteralArrays.java

Posted by Jörg Schaible <Jo...@scalaris.com>.
Gilles Sadowski wrote:

>> >> >> >> > URL: http://svn.apache.org/viewvc?rev=1245061&view=rev
>> >> >> >> > Log:
>> >> >> >> > Removed unneeded clone.
>> >> >> >> >
>> >> >> >> > The clone did not protect the array used, only the reference
>> >> >> >> > ones. JIRA: MATH-650
>> >> >> >>
>> >> >> >> -1
>> >> >> >>
>> >> >> >> That was the whole point of the clone - to protect the original
>> >> >> >> external data.
>> >> >> >
>> >> >> > Please (re-)explain what you mean by "protect".  Cf. my comment
>> >> >> > on the JIRA page.
>> >> >>
>> >> >> See also your comment of 30/Nov/11 00:31.
>> >> >
>> >> > I know, but my latest comment overrides that one.
>> >> >
>> >> >> The arrays in FastMathLiteralArrays are private, but the access
>> >> >> methods are not, and returning an array allows the caller to modify
>> >> >> array elements.
>> >> >
>> >> > It's true, but unless I'm mistaken, it doesn't matter since in the
>> >> > end there is one and only one array that will be _used_ (the
>> >> > modified one) and having a pristine copy somewhere else will not
>> >> > prevent the dire consequences of using the modified one... ;-/
>> >>
>> >> Not sure I follow that.
>> >>
>> >> Without clone, the array entries can be changed at any time.
>> >>
>> >> With clone, there are two pristine copies; neither can be changed
>> >> directly as they are stored in private arrays.
>> >
>> > The question is: Why calling "clone"?
>> > The answer is: You get a copy and you cannot change the array stored in
>> > "FastMathLiteralArrays".
>> > But that doesn't save you from the bug that consists in changing an
>> > array entry from within "FastMath".
>> > If that potential bug exists, some methods will produce erroneous
>> > values because they use the copy, not the pristine original array
>> > stored in "FastMathLiteralArrays". [After loading, the original array
>> > is never accessed again.]
>> >
>> > Now if you just obtain a reference to the original array (i.e. no call
>> > to "clone"), the bug will indeed modify it.
>> > But the consequences are not worse than in the above scenario: The
>> > exact same erroneous values will be computed.
>> 
>> Not strictly true, because it's not only the FastMath class that can
>> corrupt the array.
> 
> Only "our" classes (in the package "o.a.c.m.util") can access (and
> corrupt) the arrays.
> 
>> 
>> Of course clone does not protect against bugs in FastMath; only array
>> entry getters can do that.
> 
> Yes. That's what I said.
> 
>> But that's not the point;
> 
> Yes, that's the point.
> 
>> since the arrays were (at your insistence)
>> moved out of FastMath itself, this necessarily meant exposing the
>> contents to some degree.
>> Using clone closes that potential exposure - as would using array entry
>> getters.
> 
> "clone" provides protection against CM developers, which I deem
> unnecessary, since those same developers would also be able to make the
> "mistake" of modifying the contents "FastMathLiteralArrays". [And the same
> mistake could also happen if the arrays were stored inside the "FastMath"
> class.]

I support Gilles here, the methods are package scoped and therefore 
internal. A CM user will have to create an own package with same name (which 
will not work for sealed jars or with OSGi bundles anyway) or use reflection 
to access them. At that stage he should already know what he does. For CM 
itself there's no need to pay the penalty to create a copy.

- Jörg


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


Re: svn commit: r1245061 - /commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/FastMathLiteralArrays.java

Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
> >> >> >> > URL: http://svn.apache.org/viewvc?rev=1245061&view=rev
> >> >> >> > Log:
> >> >> >> > Removed unneeded clone.
> >> >> >> >
> >> >> >> > The clone did not protect the array used, only the reference ones.
> >> >> >> > JIRA: MATH-650
> >> >> >>
> >> >> >> -1
> >> >> >>
> >> >> >> That was the whole point of the clone - to protect the original external data.
> >> >> >
> >> >> > Please (re-)explain what you mean by "protect".  Cf. my comment on the
> >> >> > JIRA page.
> >> >>
> >> >> See also your comment of 30/Nov/11 00:31.
> >> >
> >> > I know, but my latest comment overrides that one.
> >> >
> >> >> The arrays in FastMathLiteralArrays are private, but the access
> >> >> methods are not, and returning an array allows the caller to modify
> >> >> array elements.
> >> >
> >> > It's true, but unless I'm mistaken, it doesn't matter since in the end there
> >> > is one and only one array that will be _used_ (the modified one) and having
> >> > a pristine copy somewhere else will not prevent the dire consequences of
> >> > using the modified one... ;-/
> >>
> >> Not sure I follow that.
> >>
> >> Without clone, the array entries can be changed at any time.
> >>
> >> With clone, there are two pristine copies; neither can be changed
> >> directly as they are stored in private arrays.
> >
> > The question is: Why calling "clone"?
> > The answer is: You get a copy and you cannot change the array stored in
> > "FastMathLiteralArrays".
> > But that doesn't save you from the bug that consists in changing an array
> > entry from within "FastMath".
> > If that potential bug exists, some methods will produce erroneous values
> > because they use the copy, not the pristine original array stored in
> > "FastMathLiteralArrays". [After loading, the original array is never
> > accessed again.]
> >
> > Now if you just obtain a reference to the original array (i.e. no call to
> > "clone"), the bug will indeed modify it.
> > But the consequences are not worse than in the above scenario: The exact
> > same erroneous values will be computed.
> 
> Not strictly true, because it's not only the FastMath class that can
> corrupt the array.

Only "our" classes (in the package "o.a.c.m.util") can access (and corrupt)
the arrays.

> 
> Of course clone does not protect against bugs in FastMath; only array
> entry getters can do that.

Yes. That's what I said.

> But that's not the point;

Yes, that's the point.

> since the arrays were (at your insistence)
> moved out of FastMath itself, this necessarily meant exposing the
> contents to some degree.
> Using clone closes that potential exposure - as would using array entry getters.

"clone" provides protection against CM developers, which I deem unnecessary,
since those same developers would also be able to make the "mistake" of
modifying the contents "FastMathLiteralArrays". [And the same mistake could
also happen if the arrays were stored inside the "FastMath" class.]


Gilles

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


Re: svn commit: r1245061 - /commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/FastMathLiteralArrays.java

Posted by sebb <se...@gmail.com>.
On 23 February 2012 13:43, Gilles Sadowski <gi...@harfang.homelinux.org> wrote:
> On Thu, Feb 23, 2012 at 12:47:15PM +0000, sebb wrote:
>> On 23 February 2012 12:10, Gilles Sadowski <gi...@harfang.homelinux.org> wrote:
>> > On Thu, Feb 23, 2012 at 11:32:54AM +0000, sebb wrote:
>> >> On 23 February 2012 11:19, Gilles Sadowski <gi...@harfang.homelinux.org> wrote:
>> >> > On Thu, Feb 23, 2012 at 10:55:51AM +0000, sebb wrote:
>> >> >> On 16 February 2012 16:17,  <lu...@apache.org> wrote:
>> >> >> > Author: luc
>> >> >> > Date: Thu Feb 16 16:17:14 2012
>> >> >> > New Revision: 1245061
>> >> >> >
>> >> >> > URL: http://svn.apache.org/viewvc?rev=1245061&view=rev
>> >> >> > Log:
>> >> >> > Removed unneeded clone.
>> >> >> >
>> >> >> > The clone did not protect the array used, only the reference ones.
>> >> >> > JIRA: MATH-650
>> >> >>
>> >> >> -1
>> >> >>
>> >> >> That was the whole point of the clone - to protect the original external data.
>> >> >
>> >> > Please (re-)explain what you mean by "protect".  Cf. my comment on the
>> >> > JIRA page.
>> >>
>> >> See also your comment of 30/Nov/11 00:31.
>> >
>> > I know, but my latest comment overrides that one.
>> >
>> >> The arrays in FastMathLiteralArrays are private, but the access
>> >> methods are not, and returning an array allows the caller to modify
>> >> array elements.
>> >
>> > It's true, but unless I'm mistaken, it doesn't matter since in the end there
>> > is one and only one array that will be _used_ (the modified one) and having
>> > a pristine copy somewhere else will not prevent the dire consequences of
>> > using the modified one... ;-/
>>
>> Not sure I follow that.
>>
>> Without clone, the array entries can be changed at any time.
>>
>> With clone, there are two pristine copies; neither can be changed
>> directly as they are stored in private arrays.
>
> The question is: Why calling "clone"?
> The answer is: You get a copy and you cannot change the array stored in
> "FastMathLiteralArrays".
> But that doesn't save you from the bug that consists in changing an array
> entry from within "FastMath".
> If that potential bug exists, some methods will produce erroneous values
> because they use the copy, not the pristine original array stored in
> "FastMathLiteralArrays". [After loading, the original array is never
> accessed again.]
>
> Now if you just obtain a reference to the original array (i.e. no call to
> "clone"), the bug will indeed modify it.
> But the consequences are not worse than in the above scenario: The exact
> same erroneous values will be computed.

Not strictly true, because it's not only the FastMath class that can
corrupt the array.

Of course clone does not protect against bugs in FastMath; only array
entry getters can do that.

But that's not the point; since the arrays were (at your insistence)
moved out of FastMath itself, this necessarily meant exposing the
contents to some degree.
Using clone closes that potential exposure - as would using array entry getters.

>>
>> >> If you don't want the memory overhead of a clone, then one could use
>> >> array entry getters instead.
>> >
>> > It's not that: It is because "clone" gives a false sense of security.
>>
>> How so?
>
> Cf. above.
>
>>
>> Are you saying that someone can change the original array entries by
>> accessing the cloned copy?
>
> Certainly not!
>
> Gilles
>
>> > Yes, getters would be _really_ secure, as they would prevent an inadvertant
>> > CM developer to commit a bug in the "FastMath" class. But a _user_ of
>> > "FastMath" can anyways never access the arrays (as the references/copies are
>> > "private" to "FastMath"), so I was wondering whether the additional layer of
>> > security was really worth it (in case a performance loss would accompany the
>> > getters; if not, then fine, fine).
>> >
>> >
>> > Gilles
>> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>

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


Re: svn commit: r1245061 - /commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/FastMathLiteralArrays.java

Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
On Thu, Feb 23, 2012 at 12:47:15PM +0000, sebb wrote:
> On 23 February 2012 12:10, Gilles Sadowski <gi...@harfang.homelinux.org> wrote:
> > On Thu, Feb 23, 2012 at 11:32:54AM +0000, sebb wrote:
> >> On 23 February 2012 11:19, Gilles Sadowski <gi...@harfang.homelinux.org> wrote:
> >> > On Thu, Feb 23, 2012 at 10:55:51AM +0000, sebb wrote:
> >> >> On 16 February 2012 16:17,  <lu...@apache.org> wrote:
> >> >> > Author: luc
> >> >> > Date: Thu Feb 16 16:17:14 2012
> >> >> > New Revision: 1245061
> >> >> >
> >> >> > URL: http://svn.apache.org/viewvc?rev=1245061&view=rev
> >> >> > Log:
> >> >> > Removed unneeded clone.
> >> >> >
> >> >> > The clone did not protect the array used, only the reference ones.
> >> >> > JIRA: MATH-650
> >> >>
> >> >> -1
> >> >>
> >> >> That was the whole point of the clone - to protect the original external data.
> >> >
> >> > Please (re-)explain what you mean by "protect".  Cf. my comment on the
> >> > JIRA page.
> >>
> >> See also your comment of 30/Nov/11 00:31.
> >
> > I know, but my latest comment overrides that one.
> >
> >> The arrays in FastMathLiteralArrays are private, but the access
> >> methods are not, and returning an array allows the caller to modify
> >> array elements.
> >
> > It's true, but unless I'm mistaken, it doesn't matter since in the end there
> > is one and only one array that will be _used_ (the modified one) and having
> > a pristine copy somewhere else will not prevent the dire consequences of
> > using the modified one... ;-/
> 
> Not sure I follow that.
> 
> Without clone, the array entries can be changed at any time.
> 
> With clone, there are two pristine copies; neither can be changed
> directly as they are stored in private arrays.

The question is: Why calling "clone"?
The answer is: You get a copy and you cannot change the array stored in
"FastMathLiteralArrays".
But that doesn't save you from the bug that consists in changing an array
entry from within "FastMath".
If that potential bug exists, some methods will produce erroneous values
because they use the copy, not the pristine original array stored in
"FastMathLiteralArrays". [After loading, the original array is never
accessed again.]

Now if you just obtain a reference to the original array (i.e. no call to
"clone"), the bug will indeed modify it.
But the consequences are not worse than in the above scenario: The exact
same erroneous values will be computed.

> 
> >> If you don't want the memory overhead of a clone, then one could use
> >> array entry getters instead.
> >
> > It's not that: It is because "clone" gives a false sense of security.
> 
> How so?

Cf. above.

> 
> Are you saying that someone can change the original array entries by
> accessing the cloned copy?

Certainly not!

Gilles

> > Yes, getters would be _really_ secure, as they would prevent an inadvertant
> > CM developer to commit a bug in the "FastMath" class. But a _user_ of
> > "FastMath" can anyways never access the arrays (as the references/copies are
> > "private" to "FastMath"), so I was wondering whether the additional layer of
> > security was really worth it (in case a performance loss would accompany the
> > getters; if not, then fine, fine).
> >
> >
> > Gilles
> >

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


Re: svn commit: r1245061 - /commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/FastMathLiteralArrays.java

Posted by sebb <se...@gmail.com>.
On 23 February 2012 12:10, Gilles Sadowski <gi...@harfang.homelinux.org> wrote:
> On Thu, Feb 23, 2012 at 11:32:54AM +0000, sebb wrote:
>> On 23 February 2012 11:19, Gilles Sadowski <gi...@harfang.homelinux.org> wrote:
>> > On Thu, Feb 23, 2012 at 10:55:51AM +0000, sebb wrote:
>> >> On 16 February 2012 16:17,  <lu...@apache.org> wrote:
>> >> > Author: luc
>> >> > Date: Thu Feb 16 16:17:14 2012
>> >> > New Revision: 1245061
>> >> >
>> >> > URL: http://svn.apache.org/viewvc?rev=1245061&view=rev
>> >> > Log:
>> >> > Removed unneeded clone.
>> >> >
>> >> > The clone did not protect the array used, only the reference ones.
>> >> > JIRA: MATH-650
>> >>
>> >> -1
>> >>
>> >> That was the whole point of the clone - to protect the original external data.
>> >
>> > Please (re-)explain what you mean by "protect".  Cf. my comment on the
>> > JIRA page.
>>
>> See also your comment of 30/Nov/11 00:31.
>
> I know, but my latest comment overrides that one.
>
>> The arrays in FastMathLiteralArrays are private, but the access
>> methods are not, and returning an array allows the caller to modify
>> array elements.
>
> It's true, but unless I'm mistaken, it doesn't matter since in the end there
> is one and only one array that will be _used_ (the modified one) and having
> a pristine copy somewhere else will not prevent the dire consequences of
> using the modified one... ;-/

Not sure I follow that.

Without clone, the array entries can be changed at any time.

With clone, there are two pristine copies; neither can be changed
directly as they are stored in private arrays.

>> If you don't want the memory overhead of a clone, then one could use
>> array entry getters instead.
>
> It's not that: It is because "clone" gives a false sense of security.

How so?

Are you saying that someone can change the original array entries by
accessing the cloned copy?

> Yes, getters would be _really_ secure, as they would prevent an inadvertant
> CM developer to commit a bug in the "FastMath" class. But a _user_ of
> "FastMath" can anyways never access the arrays (as the references/copies are
> "private" to "FastMath"), so I was wondering whether the additional layer of
> security was really worth it (in case a performance loss would accompany the
> getters; if not, then fine, fine).
>
>
> Gilles
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>

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


Re: svn commit: r1245061 - /commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/FastMathLiteralArrays.java

Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
On Thu, Feb 23, 2012 at 11:32:54AM +0000, sebb wrote:
> On 23 February 2012 11:19, Gilles Sadowski <gi...@harfang.homelinux.org> wrote:
> > On Thu, Feb 23, 2012 at 10:55:51AM +0000, sebb wrote:
> >> On 16 February 2012 16:17,  <lu...@apache.org> wrote:
> >> > Author: luc
> >> > Date: Thu Feb 16 16:17:14 2012
> >> > New Revision: 1245061
> >> >
> >> > URL: http://svn.apache.org/viewvc?rev=1245061&view=rev
> >> > Log:
> >> > Removed unneeded clone.
> >> >
> >> > The clone did not protect the array used, only the reference ones.
> >> > JIRA: MATH-650
> >>
> >> -1
> >>
> >> That was the whole point of the clone - to protect the original external data.
> >
> > Please (re-)explain what you mean by "protect".  Cf. my comment on the
> > JIRA page.
> 
> See also your comment of 30/Nov/11 00:31.

I know, but my latest comment overrides that one.

> The arrays in FastMathLiteralArrays are private, but the access
> methods are not, and returning an array allows the caller to modify
> array elements.

It's true, but unless I'm mistaken, it doesn't matter since in the end there
is one and only one array that will be _used_ (the modified one) and having
a pristine copy somewhere else will not prevent the dire consequences of
using the modified one... ;-/

> If you don't want the memory overhead of a clone, then one could use
> array entry getters instead.

It's not that: It is because "clone" gives a false sense of security.

Yes, getters would be _really_ secure, as they would prevent an inadvertant
CM developer to commit a bug in the "FastMath" class. But a _user_ of
"FastMath" can anyways never access the arrays (as the references/copies are
"private" to "FastMath"), so I was wondering whether the additional layer of
security was really worth it (in case a performance loss would accompany the
getters; if not, then fine, fine).


Gilles

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


Re: svn commit: r1245061 - /commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/FastMathLiteralArrays.java

Posted by sebb <se...@gmail.com>.
On 23 February 2012 11:19, Gilles Sadowski <gi...@harfang.homelinux.org> wrote:
> On Thu, Feb 23, 2012 at 10:55:51AM +0000, sebb wrote:
>> On 16 February 2012 16:17,  <lu...@apache.org> wrote:
>> > Author: luc
>> > Date: Thu Feb 16 16:17:14 2012
>> > New Revision: 1245061
>> >
>> > URL: http://svn.apache.org/viewvc?rev=1245061&view=rev
>> > Log:
>> > Removed unneeded clone.
>> >
>> > The clone did not protect the array used, only the reference ones.
>> > JIRA: MATH-650
>>
>> -1
>>
>> That was the whole point of the clone - to protect the original external data.
>
> Please (re-)explain what you mean by "protect".  Cf. my comment on the
> JIRA page.

See also your comment of 30/Nov/11 00:31.

The arrays in FastMathLiteralArrays are private, but the access
methods are not, and returning an array allows the caller to modify
array elements.

If you don't want the memory overhead of a clone, then one could use
array entry getters instead.

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


Re: svn commit: r1245061 - /commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/FastMathLiteralArrays.java

Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
On Thu, Feb 23, 2012 at 10:55:51AM +0000, sebb wrote:
> On 16 February 2012 16:17,  <lu...@apache.org> wrote:
> > Author: luc
> > Date: Thu Feb 16 16:17:14 2012
> > New Revision: 1245061
> >
> > URL: http://svn.apache.org/viewvc?rev=1245061&view=rev
> > Log:
> > Removed unneeded clone.
> >
> > The clone did not protect the array used, only the reference ones.
> > JIRA: MATH-650
> 
> -1
> 
> That was the whole point of the clone - to protect the original external data.

Please (re-)explain what you mean by "protect".  Cf. my comment on the
JIRA page.


Gilles

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


Re: svn commit: r1245061 - /commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/FastMathLiteralArrays.java

Posted by sebb <se...@gmail.com>.
On 16 February 2012 16:17,  <lu...@apache.org> wrote:
> Author: luc
> Date: Thu Feb 16 16:17:14 2012
> New Revision: 1245061
>
> URL: http://svn.apache.org/viewvc?rev=1245061&view=rev
> Log:
> Removed unneeded clone.
>
> The clone did not protect the array used, only the reference ones.
> JIRA: MATH-650

-1

That was the whole point of the clone - to protect the original external data.

> Modified:
>    commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/FastMathLiteralArrays.java
>
> Modified: commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/FastMathLiteralArrays.java
> URL: http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/FastMathLiteralArrays.java?rev=1245061&r1=1245060&r2=1245061&view=diff
> ==============================================================================
> --- commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/FastMathLiteralArrays.java (original)
> +++ commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/FastMathLiteralArrays.java Thu Feb 16 16:17:14 2012
> @@ -6139,7 +6139,7 @@ class FastMathLiteralArrays {
>      * @return a clone of the data array.
>      */
>     static double[] loadExpIntA() {
> -        return EXP_INT_A.clone();
> +        return EXP_INT_A;
>     }
>     /**
>      * Load "EXP_INT_B".
> @@ -6147,7 +6147,7 @@ class FastMathLiteralArrays {
>      * @return a clone of the data array.
>      */
>     static double[] loadExpIntB() {
> -        return EXP_INT_B.clone();
> +        return EXP_INT_B;
>     }
>     /**
>      * Load "EXP_FRAC_A".
> @@ -6155,7 +6155,7 @@ class FastMathLiteralArrays {
>      * @return a clone of the data array.
>      */
>     static double[] loadExpFracA() {
> -        return EXP_FRAC_A.clone();
> +        return EXP_FRAC_A;
>     }
>     /**
>      * Load "EXP_FRAC_B".
> @@ -6163,7 +6163,7 @@ class FastMathLiteralArrays {
>      * @return a clone of the data array.
>      */
>     static double[] loadExpFracB() {
> -        return EXP_FRAC_B.clone();
> +        return EXP_FRAC_B;
>     }
>     /**
>      * Load "LN_MANT".
> @@ -6171,6 +6171,6 @@ class FastMathLiteralArrays {
>      * @return a clone of the data array.
>      */
>     static double[][] loadLnMant() {
> -        return LN_MANT.clone();
> +        return LN_MANT;
>     }
>  }
>
>

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