You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by kk...@apache.org on 2014/04/18 17:29:06 UTC
svn propchange: r1588102 - svn:log
Author: kkolinko
Revision: 1588102
Modified property: svn:log
Modified: svn:log at Fri Apr 18 15:29:06 2014
------------------------------------------------------------------------------
--- svn:log (original)
+++ svn:log Fri Apr 18 15:29:06 2014
@@ -1 +1,3 @@
Fix an Eclipse nag
+Correct signature of SSL.fipsModeGet(),
+as native code is implemented as throwing an Exception in this method.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn propchange: r1588102 - svn:log
Posted by Konstantin Kolinko <kn...@gmail.com>.
2014-04-22 23:53 GMT+04:00 Christopher Schultz <ch...@christopherschultz.net>:
> Mark/Konstantin,
>
> On 4/18/14, 11:29 AM, kkolinko@apache.org wrote:
>> Author: kkolinko
>> Revision: 1588102
>> Modified property: svn:log
>>
>> Modified: svn:log at Fri Apr 18 15:29:06 2014
>> ------------------------------------------------------------------------------
>> --- svn:log (original)
>> +++ svn:log Fri Apr 18 15:29:06 2014
>> @@ -1 +1,3 @@
>> Fix an Eclipse nag
>> +Correct signature of SSL.fipsModeGet(),
>> +as native code is implemented as throwing an Exception in this method.
>
> Why single-out this method only to have its method signature changed?
>
> Nearly every native method in SSL.java can throw an exception. I was
> just following convention.
Good catch.
Then those other methods that throw exceptions in their native code
need them documented and their signatures fixed as well.
E.g. "fipsModeSet()"
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
RE: svn propchange: r1588102 - svn:log
Posted by Konstantin Preißer <kp...@apache.org>.
Hi,
> -----Original Message-----
> From: Konstantin Kolinko [mailto:knst.kolinko@gmail.com]
> Sent: Tuesday, April 22, 2014 10:50 PM
>
> 2014-04-23 0:43 GMT+04:00 Mark Thomas <ma...@apache.org>:
(snip)
> > No objection to the final being restored, it is good practice to use
> > final when possible.
> >
> > It isn't something that has been used much elsewhere in the Tomcat code
> > base. Is there much benefit to going through and adding it everywhere it
> > could be used? If I recall correctly, the UCDetector plugin can flag up
> > all the places where this can be done and probably add them as well.
>
> I think that it does not change the generated byte code, thus no real
> difference.
> It is good from documentation point of view (to express you intention
> to do not change the variable).
>
> If one goes pedantic way, I have seen code where 'final' was added to
> all parameters of every method. I do not like that code style.
+1
I've once worked with a Java program at a company that used a code style tool issuing a warning for every local variable that did not change and was not declared as "final". The result was that for some method's code, you felt like the code consists only of "final" modifiers and it was harder to read.
AFAIK, the only two technical uses of "final" for method/local variables are:
1) If you assign a constant expression to a variable, the Eclipse and Oracle compiler will replace references to this variable with the constant expression. E.g., if you write:
public static void someMethod(int x) {
int i1 = 42;
final int i2 = 42;
System.out.println(i1);
System.out.println(i2);
int x1 = x;
final int x2 = x;
System.out.println(x1);
System.out.println(x2);
}
and compile it, then view the .class file with a decompiler, it will show:
public static void someMethod(int x) {
int i1 = 42;
int i2 = 42;
System.out.println(i1);
System.out.println(42);
int x1 = x;
int x2 = x;
System.out.println(x1);
System.out.println(x2);
}
So references to "i2" have been replaced with the constant expression by the compiler. This should also happen for class attributes that a "final" modifier and constant values.
2) If you want to use a local variable inside of an anonymous inner class. The compiler will have to make a copy of the variable when the anonymous class instance is created, as the variable is stored in the stack and will vanish once the outer method returns. I think the "final" probably would technically not be needed, but then if you changed the variable in the anonymous inner class' method, the change would not be reflected in the outer method.
For comparison, in .Net/C#, when you write an anonymous method inside of another method and the anonymous method accesses some variables of the outer method, the compiler will automatically change the code to store that set of variables in some wrapper object, and have the code working with that wrapper. That means if you change one of the variables in the anonymous method, the change will be reflected on the outer method.
Consequently, C# does have a "readonly" modifier to mark class/type attributes as read-only like the "final" in Java, but not for local variables. However, for constant values, it has the "const" modifier that can be used for both class-wide variables (which would be like using "static readonly") and local variables, so the compiler will replace those variable references with the constant values (and "const" variables will not take space on the stack or in an object).
Regards,
Konstantin Preißer
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn propchange: r1588102 - svn:log
Posted by Konstantin Kolinko <kn...@gmail.com>.
2014-04-23 0:43 GMT+04:00 Mark Thomas <ma...@apache.org>:
> On 22/04/2014 21:35, Konstantin Kolinko wrote:
>> 2014-04-23 0:30 GMT+04:00 Mark Thomas <ma...@apache.org>:
>>> On 22/04/2014 21:21, Christopher Schultz wrote:
>>>
>>>> Before posting, I checked just so I wouldn't embarrass myself. It
>>>> worked just fine. My guess is that you changed the implementation
>>>> to throw an exception after removing the 'final' which then makes
>>>> the 'final' legal again.
>>>
>>> I don't see any variation that includes final that compiles.
>>>
>>> Option 1:
>>>
>>> int final fipsModeState;
>>> try {
>>> fipsModeState = SSL.fipsModeGet();
>>> } catch (Exception e) {
>>> throw new IllegalStateException(e);
>>> }
>>>
>>>
>>> This failure looks correct to me since it should be illegal to change
>>> the value of a final variable after it has been declared.
>>
>> 1. s/ declared / assigned /
>>
>> 2. The syntax is
>> final int fipsModeState;
>>
>> ;)
>
> Doh!
>
> No objection to the final being restored, it is good practice to use
> final when possible.
>
> It isn't something that has been used much elsewhere in the Tomcat code
> base. Is there much benefit to going through and adding it everywhere it
> could be used? If I recall correctly, the UCDetector plugin can flag up
> all the places where this can be done and probably add them as well.
I think that it does not change the generated byte code, thus no real
difference.
It is good from documentation point of view (to express you intention
to do not change the variable).
If one goes pedantic way, I have seen code where 'final' was added to
all parameters of every method. I do not like that code style.
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn propchange: r1588102 - svn:log
Posted by Mark Thomas <ma...@apache.org>.
On 22/04/2014 21:35, Konstantin Kolinko wrote:
> 2014-04-23 0:30 GMT+04:00 Mark Thomas <ma...@apache.org>:
>> On 22/04/2014 21:21, Christopher Schultz wrote:
>>
>>> Before posting, I checked just so I wouldn't embarrass myself. It
>>> worked just fine. My guess is that you changed the implementation
>>> to throw an exception after removing the 'final' which then makes
>>> the 'final' legal again.
>>
>> I don't see any variation that includes final that compiles.
>>
>> Option 1:
>>
>> int final fipsModeState;
>> try {
>> fipsModeState = SSL.fipsModeGet();
>> } catch (Exception e) {
>> throw new IllegalStateException(e);
>> }
>>
>>
>> This failure looks correct to me since it should be illegal to change
>> the value of a final variable after it has been declared.
>
> 1. s/ declared / assigned /
>
> 2. The syntax is
> final int fipsModeState;
>
> ;)
Doh!
No objection to the final being restored, it is good practice to use
final when possible.
It isn't something that has been used much elsewhere in the Tomcat code
base. Is there much benefit to going through and adding it everywhere it
could be used? If I recall correctly, the UCDetector plugin can flag up
all the places where this can be done and probably add them as well.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn propchange: r1588102 - svn:log
Posted by Konstantin Kolinko <kn...@gmail.com>.
2014-04-23 0:30 GMT+04:00 Mark Thomas <ma...@apache.org>:
> On 22/04/2014 21:21, Christopher Schultz wrote:
>
>> Before posting, I checked just so I wouldn't embarrass myself. It
>> worked just fine. My guess is that you changed the implementation
>> to throw an exception after removing the 'final' which then makes
>> the 'final' legal again.
>
> I don't see any variation that includes final that compiles.
>
> Option 1:
>
> int final fipsModeState;
> try {
> fipsModeState = SSL.fipsModeGet();
> } catch (Exception e) {
> throw new IllegalStateException(e);
> }
>
>
> This failure looks correct to me since it should be illegal to change
> the value of a final variable after it has been declared.
1. s/ declared / assigned /
2. The syntax is
final int fipsModeState;
;)
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn propchange: r1588102 - svn:log
Posted by Christopher Schultz <ch...@christopherschultz.net>.
Mark,
On 4/22/14, 4:30 PM, Mark Thomas wrote:
> On 22/04/2014 21:21, Christopher Schultz wrote:
>
>> Before posting, I checked just so I wouldn't embarrass myself. It
>> worked just fine. My guess is that you changed the implementation
>> to throw an exception after removing the 'final' which then makes
>> the 'final' legal again.
>
> I don't see any variation that includes final that compiles.
>
> Option 1:
>
> int final fipsModeState;
> try {
> fipsModeState = SSL.fipsModeGet();
> } catch (Exception e) {
> throw new IllegalStateException(e);
> }
>
>
> This failure looks correct to me since it should be illegal to change
> the value of a final variable after it has been declared.
>
>
> Option 2:
> try {
> int final fipsModeState = SSL.fipsModeGet();
> } catch (Exception e) {
> throw new IllegalStateException(e);
> }
>
> This code snippet it OK but compilation fails as the scope of
> fipsModeState is now limited to the try block and it is needed outside
> of that so again, compilation fails.
>
>
> How do you propose to mark fipsModeState as final and still have the
> code compile?
$ cat FinalTest.java
public class FinalTest {
public static void main(String[] args) {
final int fipsModeState;
try {
fipsModeState = 1;
} catch (Exception e) {
// Do nothing
}
}
}
$ javac -J-showversion FinalTest.java
java version "1.7.0_55"
Java(TM) SE Runtime Environment (build 1.7.0_55-b13)
Java HotSpot(TM) 64-Bit Server VM (build 24.55-b03, mixed mode)
[no warnings. no errors]
Honestly, I was surprised to see that the compiler allows a code path
where fipsModeState was not set. It seems that the compiler allows you
to get by until you try to actually use the value, at which point you'll
get an "uninitialized local" error.
-chris
Re: svn propchange: r1588102 - svn:log
Posted by Mark Thomas <ma...@apache.org>.
On 22/04/2014 21:21, Christopher Schultz wrote:
> Before posting, I checked just so I wouldn't embarrass myself. It
> worked just fine. My guess is that you changed the implementation
> to throw an exception after removing the 'final' which then makes
> the 'final' legal again.
I don't see any variation that includes final that compiles.
Option 1:
int final fipsModeState;
try {
fipsModeState = SSL.fipsModeGet();
} catch (Exception e) {
throw new IllegalStateException(e);
}
This failure looks correct to me since it should be illegal to change
the value of a final variable after it has been declared.
Option 2:
try {
int final fipsModeState = SSL.fipsModeGet();
} catch (Exception e) {
throw new IllegalStateException(e);
}
This code snippet it OK but compilation fails as the scope of
fipsModeState is now limited to the try block and it is needed outside
of that so again, compilation fails.
How do you propose to mark fipsModeState as final and still have the
code compile?
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn propchange: r1588102 - svn:log
Posted by Christopher Schultz <ch...@christopherschultz.net>.
Mark,
On 4/22/14, 4:18 PM, Mark Thomas wrote:
> On 22/04/2014 21:15, Christopher Schultz wrote:
>> Mark,
>
>> On 4/22/14, 3:58 PM, Mark Thomas wrote:
>>> On 22/04/2014 20:53, Christopher Schultz wrote:
>>>> Mark/Konstantin,
>>>
>>>> On 4/18/14, 11:29 AM, kkolinko@apache.org wrote:
>>>>> Author: kkolinko Revision: 1588102 Modified property:
>>>>> svn:log
>>>>>
>>>>> Modified: svn:log at Fri Apr 18 15:29:06 2014
>>>>> ------------------------------------------------------------------------------
>>>>>
>>>>>
>>>
>>>>>
> --- svn:log (original)
>>>>> +++ svn:log Fri Apr 18 15:29:06 2014 @@ -1 +1,3 @@ Fix an
>>>>> Eclipse nag +Correct signature of SSL.fipsModeGet(), +as
>>>>> native code is implemented as throwing an Exception in this
>>>>> method.
>>>
>>>> Why single-out this method only to have its method signature
>>>> changed?
>>>
>>>> Nearly every native method in SSL.java can throw an exception.
>>>> I was just following convention.
>>>
>>> Because you documented the Exception in the Javadoc without
>>> declaring it in the code and that triggered an IDE warning. The
>>> 8.0.x code is kept warning free so either the Javadoc had to be
>>> removed or the declaration corrected. Correcting the declaration
>>> seemed to be the better option since it is possible for an
>>> exception to be thrown in this case.
>
>> Fair enough. Shall we change the other methods in SSL.java as
>> well?
>
> That is worth doing but I'd do that as a separate clean up task.
>
>> Any particular reason you removed the 'final' from fipsModeState?
>
> Yes. If the reason is not immediately obvious to you I suggest you
> restore it and then try and compile the class.
Before posting, I checked just so I wouldn't embarrass myself. It worked
just fine. My guess is that you changed the implementation to throw an
exception after removing the 'final' which then makes the 'final' legal
again.
-chris
Re: svn propchange: r1588102 - svn:log
Posted by Mark Thomas <ma...@apache.org>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 22/04/2014 21:15, Christopher Schultz wrote:
> Mark,
>
> On 4/22/14, 3:58 PM, Mark Thomas wrote:
>> On 22/04/2014 20:53, Christopher Schultz wrote:
>>> Mark/Konstantin,
>>
>>> On 4/18/14, 11:29 AM, kkolinko@apache.org wrote:
>>>> Author: kkolinko Revision: 1588102 Modified property:
>>>> svn:log
>>>>
>>>> Modified: svn:log at Fri Apr 18 15:29:06 2014
>>>> ------------------------------------------------------------------------------
>>>>
>>>>
>>
>>>>
- --- svn:log (original)
>>>> +++ svn:log Fri Apr 18 15:29:06 2014 @@ -1 +1,3 @@ Fix an
>>>> Eclipse nag +Correct signature of SSL.fipsModeGet(), +as
>>>> native code is implemented as throwing an Exception in this
>>>> method.
>>
>>> Why single-out this method only to have its method signature
>>> changed?
>>
>>> Nearly every native method in SSL.java can throw an exception.
>>> I was just following convention.
>>
>> Because you documented the Exception in the Javadoc without
>> declaring it in the code and that triggered an IDE warning. The
>> 8.0.x code is kept warning free so either the Javadoc had to be
>> removed or the declaration corrected. Correcting the declaration
>> seemed to be the better option since it is possible for an
>> exception to be thrown in this case.
>
> Fair enough. Shall we change the other methods in SSL.java as
> well?
That is worth doing but I'd do that as a separate clean up task.
> Any particular reason you removed the 'final' from fipsModeState?
Yes. If the reason is not immediately obvious to you I suggest you
restore it and then try and compile the class.
Mark
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (MingW32)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
iQIcBAEBAgAGBQJTVs6mAAoJEBDAHFovYFnnivYQAKQDeQa4bFO0ewqFhQz9UQt1
zPYIBzK/grOID+SPdPLWbmL4bq2G4K1PuZFvvjwfGYzdYVFoLZ0a5sauGc3WWv2/
seGQ00kqOLCr4SiXslvyYTq35hewoJTcrCONzQ880IHqQ5VgrB0G2ZAKewpJBX+S
6bvr90CZLgBTP/Myl16RkBr6fW5y2yOIh6RMnYwbCby+Nw7fqxzsNTdCFyvtfpIQ
G/41Xa3c2msNL+0uReKrOBM9fgN9Y/HPIWc5NTlqHC/RB9RcmX9C6pP/GSomIwBs
XEJ8WUoLfvc2ynH0HjWw33Cu2/+IwlG8xoubJE2ragOBSKGOCHUle2SNF7/1/itc
vZsR/G3GB46BuMFMShQ726mEX3g4UFnriAg+9Jz+s5CMx64QPgro1cVWVsgXTb6v
DUNDQkzAAaATxeurvaog6B8Yo3HIGONk5IooA/PwyN4rDTDhaoWYGNfleZvVfbxt
98Mt1T5Dbkr/bqhxb5Rwht3al7gUMZ2RGxf1rhid5AFv8pdcPcpqLU7jVzfNFo46
1CMpO8HYpMe1Wa1nfeN5Yjl3h5YiqMCDhnse23JYr1N9m0JvLy73EQJE1vuPnr7T
K1ngelviByJom3SAuxjPS7EfVVDn1k2HLwn7gKz1iO971tANyrm12i/MHRq66v/1
DqmvW9ZsUQSxtfLNgsAy
=nMMw
-----END PGP SIGNATURE-----
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn propchange: r1588102 - svn:log
Posted by Christopher Schultz <ch...@christopherschultz.net>.
Mark,
On 4/22/14, 3:58 PM, Mark Thomas wrote:
> On 22/04/2014 20:53, Christopher Schultz wrote:
>> Mark/Konstantin,
>
>> On 4/18/14, 11:29 AM, kkolinko@apache.org wrote:
>>> Author: kkolinko Revision: 1588102 Modified property: svn:log
>>>
>>> Modified: svn:log at Fri Apr 18 15:29:06 2014
>>> ------------------------------------------------------------------------------
>>>
>>>
> --- svn:log (original)
>>> +++ svn:log Fri Apr 18 15:29:06 2014 @@ -1 +1,3 @@ Fix an Eclipse
>>> nag +Correct signature of SSL.fipsModeGet(), +as native code is
>>> implemented as throwing an Exception in this method.
>
>> Why single-out this method only to have its method signature
>> changed?
>
>> Nearly every native method in SSL.java can throw an exception. I
>> was just following convention.
>
> Because you documented the Exception in the Javadoc without declaring
> it in the code and that triggered an IDE warning. The 8.0.x code is
> kept warning free so either the Javadoc had to be removed or the
> declaration corrected. Correcting the declaration seemed to be the
> better option since it is possible for an exception to be thrown in
> this case.
Fair enough. Shall we change the other methods in SSL.java as well?
Any particular reason you removed the 'final' from fipsModeState?
-chris
Re: svn propchange: r1588102 - svn:log
Posted by Mark Thomas <ma...@apache.org>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 22/04/2014 20:53, Christopher Schultz wrote:
> Mark/Konstantin,
>
> On 4/18/14, 11:29 AM, kkolinko@apache.org wrote:
>> Author: kkolinko Revision: 1588102 Modified property: svn:log
>>
>> Modified: svn:log at Fri Apr 18 15:29:06 2014
>> ------------------------------------------------------------------------------
>>
>>
- --- svn:log (original)
>> +++ svn:log Fri Apr 18 15:29:06 2014 @@ -1 +1,3 @@ Fix an Eclipse
>> nag +Correct signature of SSL.fipsModeGet(), +as native code is
>> implemented as throwing an Exception in this method.
>
> Why single-out this method only to have its method signature
> changed?
>
> Nearly every native method in SSL.java can throw an exception. I
> was just following convention.
Because you documented the Exception in the Javadoc without declaring
it in the code and that triggered an IDE warning. The 8.0.x code is
kept warning free so either the Javadoc had to be removed or the
declaration corrected. Correcting the declaration seemed to be the
better option since it is possible for an exception to be thrown in
this case.
Mark
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (MingW32)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
iQIcBAEBAgAGBQJTVsnKAAoJEBDAHFovYFnnPDoP/Roc+9RwZnCSnZIj7Kf+Xrt9
Mu7VcrkxthxM4+QOtWXv75iQ9IWr+q2NGwwt6WbFjkfDRl3dYlMvvBwKlDYYTyZG
PSLOJ38+LK+bufCLaFF2ZuXKnlQsEwiD3Sn9EqOJljlOtLhv+BjzOIEESMzaUTPC
sgZs/d2GfiFBiYyiq6T2+zwisSbWCRw1/yE3Dv+cj1EKaC1oNlp8wv4yvap+wIRr
RcszeZeIK3uID7gezZ74hu6eODRHPOYZeqtsu4jGLD7fD9Zh0B8hK4u3o/Mu2Al5
NGYDYAXmXGwS7PJIK37AonBO9vN4tEGbRhqhn4+F+ERGt30t4crbtXVziwJKJWYQ
CV74o+nJhjd7nCUMmNqKuNzGsIKzE8b1nI61taI1pyMSa13CxXcVvDcdiCSnDqAT
bgYF9bGyPi1YWAY5JwL4J8T5PwsGc996z/LyQQaPu65UGqNblyfT7WyUO79Pg4DS
sW+eItWLiPbr1eAIEk2LEu7jlCmZ+ZQnDqzmcFSjvAQZYJwEZAIvSEDgsZ8H7TQJ
7dM/4DqQAXfi5RtsGXPmTp7V4tAE6e7zckIbJ+LZUtej6PsuApnMmTIIgoCwAkuz
cIYyaaYik6eArPUjz7zsVIOsK/PO5wYebIxNCObPCGQ2D3eoC+VzQvl3vgu2SWLW
n+5/rpv0/twEGzc13dxp
=yA+j
-----END PGP SIGNATURE-----
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn propchange: r1588102 - svn:log
Posted by Christopher Schultz <ch...@christopherschultz.net>.
Mark/Konstantin,
On 4/18/14, 11:29 AM, kkolinko@apache.org wrote:
> Author: kkolinko
> Revision: 1588102
> Modified property: svn:log
>
> Modified: svn:log at Fri Apr 18 15:29:06 2014
> ------------------------------------------------------------------------------
> --- svn:log (original)
> +++ svn:log Fri Apr 18 15:29:06 2014
> @@ -1 +1,3 @@
> Fix an Eclipse nag
> +Correct signature of SSL.fipsModeGet(),
> +as native code is implemented as throwing an Exception in this method.
Why single-out this method only to have its method signature changed?
Nearly every native method in SSL.java can throw an exception. I was
just following convention.
-chris