You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by is...@apache.org on 2018/10/04 21:07:54 UTC
svn commit: r1842849 - in /tomcat/trunk:
java/org/apache/tomcat/jni/Library.java
test/org/apache/tomcat/util/net/TesterSupport.java
test/org/apache/tomcat/util/net/openssl/TestOpenSSLConf.java
Author: isapir
Date: Thu Oct 4 21:07:54 2018
New Revision: 1842849
URL: http://svn.apache.org/viewvc?rev=1842849&view=rev
Log:
System.load() expects absolute path.
Error message was being discarded during tests so now it is logged.
Modified:
tomcat/trunk/java/org/apache/tomcat/jni/Library.java
tomcat/trunk/test/org/apache/tomcat/util/net/TesterSupport.java
tomcat/trunk/test/org/apache/tomcat/util/net/openssl/TestOpenSSLConf.java
Modified: tomcat/trunk/java/org/apache/tomcat/jni/Library.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/jni/Library.java?rev=1842849&r1=1842848&r2=1842849&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/jni/Library.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/jni/Library.java Thu Oct 4 21:07:54 2018
@@ -39,7 +39,7 @@ public final class Library {
for (int i = 0; i < NAMES.length; i++) {
File library = new File(binLib, System.mapLibraryName(NAMES[i]));
try {
- System.load(library.getPath());
+ System.load(library.getAbsolutePath());
loaded = true;
} catch (ThreadDeath t) {
throw t;
@@ -83,7 +83,7 @@ public final class Library {
throw t;
}
}
- if (i > 0) {
+ if (err.length() > 0) {
err.append(", ");
}
err.append(t.getMessage());
Modified: tomcat/trunk/test/org/apache/tomcat/util/net/TesterSupport.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/tomcat/util/net/TesterSupport.java?rev=1842849&r1=1842848&r2=1842849&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/tomcat/util/net/TesterSupport.java (original)
+++ tomcat/trunk/test/org/apache/tomcat/util/net/TesterSupport.java Thu Oct 4 21:07:54 2018
@@ -72,6 +72,7 @@ public final class TesterSupport {
public static final String LOCALHOST_KEY_PEM = SSL_DIR + "localhost-key.pem";
public static final boolean OPENSSL_AVAILABLE;
public static final int OPENSSL_VERSION;
+ public static final String OPENSSL_ERROR;
public static final String ROLE = "testrole";
@@ -82,16 +83,18 @@ public final class TesterSupport {
static {
boolean available = false;
int version = 0;
+ String err = "";
try {
Library.initialize(null);
available = true;
version = SSL.version();
Library.terminate();
} catch (Exception | LibraryNotFoundError ex) {
- // Ignore
+ err = ex.getMessage();
}
OPENSSL_AVAILABLE = available;
OPENSSL_VERSION = version;
+ OPENSSL_ERROR = err;
}
public static boolean isOpensslAvailable() {
Modified: tomcat/trunk/test/org/apache/tomcat/util/net/openssl/TestOpenSSLConf.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/tomcat/util/net/openssl/TestOpenSSLConf.java?rev=1842849&r1=1842848&r2=1842849&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/tomcat/util/net/openssl/TestOpenSSLConf.java (original)
+++ tomcat/trunk/test/org/apache/tomcat/util/net/openssl/TestOpenSSLConf.java Thu Oct 4 21:07:54 2018
@@ -87,7 +87,11 @@ public class TestOpenSSLConf extends Tom
@Test
public void testOpenSSLConfCmdCipher() throws Exception {
- log.info("Found OpenSSL version 0x" + Integer.toHexString(OPENSSL_VERSION));
+ if (TesterSupport.isOpensslAvailable())
+ log.info("Found OpenSSL version 0x" + Integer.toHexString(OPENSSL_VERSION));
+ else
+ log.warn("OpenSSL not found: " + TesterSupport.OPENSSL_ERROR);
+
SSLHostConfig sslHostConfig;
if (hasTLS13()) {
// Ensure TLSv1.3 ciphers aren't returned
@@ -106,7 +110,11 @@ public class TestOpenSSLConf extends Tom
@Test
public void testOpenSSLConfCmdProtocol() throws Exception {
- log.info("Found OpenSSL version 0x" + Integer.toHexString(OPENSSL_VERSION));
+ if (TesterSupport.isOpensslAvailable())
+ log.info("Found OpenSSL version 0x" + Integer.toHexString(OPENSSL_VERSION));
+ else
+ log.warn("OpenSSL not found: " + TesterSupport.OPENSSL_ERROR);
+
Set<String> disabledProtocols = new HashSet<>(Arrays.asList(DISABLED_PROTOCOLS));
StringBuilder sb = new StringBuilder();
for (String protocol : DISABLED_PROTOCOLS) {
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
RE: svn commit: r1842849 - in /tomcat/trunk:
java/org/apache/tomcat/jni/Library.java
test/org/apache/tomcat/util/net/TesterSupport.java
test/org/apache/tomcat/util/net/openssl/TestOpenSSLConf.java
Posted by "Caldarale, Charles R" <Ch...@unisys.com>.
> From: Igal Sapir [mailto:isapir@apache.org]
> Subject: Re: svn commit: r1842849 - in /tomcat/trunk:
java/org/apache/tomcat/jni/Library.java
> test/org/apache/tomcat/util/net/TesterSupport.java
> test/org/apache/tomcat/util/net/openssl/TestOpenSSLConf.java
> How do you feel about aligning the `else` with the `if`? I think that
> it makes it much clearer which block it refers to, e.g.
> if (a == b) {
> ...
> }
> else if (a == c) {
> ...
> }
> else if (a == d) {
> ...
> }
Please don't - besides wasting space, it still invites errors (albeit
usually caught by an IDE or compiler) when someone inserts code or comments
between the closing brace and the else. (Yes, this is one of my pet peeves,
and I'm very glad Tomcat code isn't written this way.)
- Chuck
THIS COMMUNICATION MAY CONTAIN CONFIDENTIAL AND/OR OTHERWISE PROPRIETARY
MATERIAL and is thus for use only by the intended recipient. If you received
this in error, please contact the sender and delete the e-mail and its
attachments from all computers.
Re: svn commit: r1842849 - in /tomcat/trunk:
java/org/apache/tomcat/jni/Library.java
test/org/apache/tomcat/util/net/TesterSupport.java
test/org/apache/tomcat/util/net/openssl/TestOpenSSLConf.java
Posted by Igal Sapir <is...@apache.org>.
On 10/5/2018 3:22 AM, Mark Thomas wrote:
> On 05/10/18 10:42, Rémy Maucherat wrote:
>> On Fri, Oct 5, 2018 at 11:40 AM Mark Thomas <ma...@apache.org> wrote:
>>> The Tomcat style is to always use { and } even for one line for
>>> clarity.
>>> Due to the age of the code base, there are a mix of styles. Generally,
>>> we try and move code towards the currently accepted style as we change it.
>> +1 a lack of { } is too big a possible bug source to ignore.
> I just tried enabling the CheckStyle test for this. There were just
> under three thousand errors.
>
> I'm wondering if it is worth going through the code base fixing these.
>
> On a related topic, I did notice several instance of the following:
>
> if (a == b) ...
> if (a == c) ...
> if (a == d) ...
>
> that could be more efficiently written as:
>
> if (a == b) {
> ...
> } else if (a == c) {
> ...
> } else if (a == d) {
> ...
> }
How do you feel about aligning the `else` with the `if`? I think that
it makes it much clearer which block it refers to, e.g.
if (a == b) {
...
}
else if (a == c) {
...
}
else if (a == d) {
...
}
Igal
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1842849 - in /tomcat/trunk:
java/org/apache/tomcat/jni/Library.java
test/org/apache/tomcat/util/net/TesterSupport.java
test/org/apache/tomcat/util/net/openssl/TestOpenSSLConf.java
Posted by Igal Sapir <is...@apache.org>.
On 10/8/2018 2:52 AM, Mark Thomas wrote:
> On 05/10/18 19:46, Christopher Schultz wrote:
>> Mark,
>>
>> On 10/5/18 06:22, Mark Thomas wrote:
>>> On 05/10/18 10:42, Rémy Maucherat wrote:
>>>> On Fri, Oct 5, 2018 at 11:40 AM Mark Thomas <ma...@apache.org>
>>>> wrote:
>>>>
>>>>> On 04/10/18 22:07, isapir@apache.org wrote:
>>>>>> Author: isapir Date: Thu Oct 4 21:07:54 2018 New Revision:
>>>>>> 1842849
>>>>>>
>>>>>> URL: http://svn.apache.org/viewvc?rev=1842849&view=rev Log:
>>>>>> System.load() expects absolute path.
>>>>> Remember to consider whether or not any changes you make to
>>>>> trunk should be back-ported to 8.5.x and 7.0.x. Generally,
>>>>> changes are back-ported unless they require changing a public
>>>>> API (as defined in RELEASE-NOTES) or are considering likely to
>>>>> cause a regression.
>>>>>
>>>>> <snip/>
>>>>>
>>>>> The Tomcat style is to always use { and } even for one line for
>>>>> clarity.
>>>>>
>>>>> Due to the age of the code base, there are a mix of styles.
>>>>> Generally, we try and move code towards the currently accepted
>>>>> style as we change it.
>>>>>
>>>> +1 a lack of { } is too big a possible bug source to ignore.
>>> I just tried enabling the CheckStyle test for this. There were
>>> just under three thousand errors.
>>> I'm wondering if it is worth going through the code base fixing
>>> these.
>> I'm nearly -1 on this, mostly because it will make back-porting stuff
>> a total PITA.
> Fair enough. I don't need much convincing not to do it as I have plenty
> of other stuff on my TODO list.
>
>> Definitely opportunistically "upgrade" code we find here and there,
>> but I don't think it's worth taking a day or two to add missing
>> explicit blocks everywhere.
> ACK.
>
>>> On a related topic, I did notice several instance of the
>>> following:
>>> if (a == b) ... if (a == c) ... if (a == d) ...
>>> that could be more efficiently written as:
>>> if (a == b) { ... } else if (a == c) { ... } else if (a == d) {
>>> ... }
>> That would be nice. Sounds like a BZ issue that could have a
>> "beginner" keyword attached.
> Good idea. Feel free to add that if I don't get there first.
I'd be happy to work on that.
Igal
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1842849 - in /tomcat/trunk:
java/org/apache/tomcat/jni/Library.java
test/org/apache/tomcat/util/net/TesterSupport.java
test/org/apache/tomcat/util/net/openssl/TestOpenSSLConf.java
Posted by Mark Thomas <ma...@apache.org>.
On 09/10/18 07:04, Igal Sapir wrote:
> Mark,
>
> On Mon, Oct 8, 2018 at 2:52 AM Mark Thomas <ma...@apache.org> wrote:
>
>> On 05/10/18 19:46, Christopher Schultz wrote:
>> <snip/>
>>>>> +1 a lack of { } is too big a possible bug source to ignore.
>>>
>>>> I just tried enabling the CheckStyle test for this. There were
>>>> just under three thousand errors.
>>>
>>>> I'm wondering if it is worth going through the code base fixing
>>>> these.
>>>
>>> I'm nearly -1 on this, mostly because it will make back-porting stuff
>>> a total PITA.
>>
>> Fair enough. I don't need much convincing not to do it as I have plenty
>> of other stuff on my TODO list.
>>
>>> Definitely opportunistically "upgrade" code we find here and there,
>>> but I don't think it's worth taking a day or two to add missing
>>> explicit blocks everywhere.
>>
>> ACK.
>>
>>>> On a related topic, I did notice several instance of the
>>>> following:
>>>
>>>> if (a == b) ... if (a == c) ... if (a == d) ...
>>>
>>>> that could be more efficiently written as:
>>>
>>>> if (a == b) { ... } else if (a == c) { ... } else if (a == d) {
>>>> ... }
>>>
>>> That would be nice. Sounds like a BZ issue that could have a
>>> "beginner" keyword attached.
>>
>> Good idea. Feel free to add that if I don't get there first.
>>
>>
> Did you notice the consecutive if statements by chance, or does CheckStyle
> report those?
I noticed them by chance while I was looking at CheckStyle warnings for
if statements not using { ... }
> I just imported the CheckStyle profile into IntelliJ IDEA
> and I see more than 82,000 warnings, many of which complaining of missing
> Javadoc comments and lines longer than 80 characters.
If you import the checkstyle configs from res/checkstyle.xml and apply
them as per https://github.com/apache/tomcat/blob/trunk/build.xml#L565
you should get a clean build.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1842849 - in /tomcat/trunk: java/org/apache/tomcat/jni/Library.java
test/org/apache/tomcat/util/net/TesterSupport.java test/org/apache/tomcat/util/net/openssl/TestOpenSSLConf.java
Posted by Igal Sapir <ig...@lucee.org>.
Mark,
On Mon, Oct 8, 2018 at 2:52 AM Mark Thomas <ma...@apache.org> wrote:
> On 05/10/18 19:46, Christopher Schultz wrote:
> <snip/>
> >>> +1 a lack of { } is too big a possible bug source to ignore.
> >
> >> I just tried enabling the CheckStyle test for this. There were
> >> just under three thousand errors.
> >
> >> I'm wondering if it is worth going through the code base fixing
> >> these.
> >
> > I'm nearly -1 on this, mostly because it will make back-porting stuff
> > a total PITA.
>
> Fair enough. I don't need much convincing not to do it as I have plenty
> of other stuff on my TODO list.
>
> > Definitely opportunistically "upgrade" code we find here and there,
> > but I don't think it's worth taking a day or two to add missing
> > explicit blocks everywhere.
>
> ACK.
>
> >> On a related topic, I did notice several instance of the
> >> following:
> >
> >> if (a == b) ... if (a == c) ... if (a == d) ...
> >
> >> that could be more efficiently written as:
> >
> >> if (a == b) { ... } else if (a == c) { ... } else if (a == d) {
> >> ... }
> >
> > That would be nice. Sounds like a BZ issue that could have a
> > "beginner" keyword attached.
>
> Good idea. Feel free to add that if I don't get there first.
>
>
Did you notice the consecutive if statements by chance, or does CheckStyle
report those? I just imported the CheckStyle profile into IntelliJ IDEA
and I see more than 82,000 warnings, many of which complaining of missing
Javadoc comments and lines longer than 80 characters.
Thanks,
Igal
Re: svn commit: r1842849 - in /tomcat/trunk:
java/org/apache/tomcat/jni/Library.java
test/org/apache/tomcat/util/net/TesterSupport.java
test/org/apache/tomcat/util/net/openssl/TestOpenSSLConf.java
Posted by Mark Thomas <ma...@apache.org>.
On 05/10/18 19:46, Christopher Schultz wrote:
> Mark,
>
> On 10/5/18 06:22, Mark Thomas wrote:
>> On 05/10/18 10:42, Rémy Maucherat wrote:
>>> On Fri, Oct 5, 2018 at 11:40 AM Mark Thomas <ma...@apache.org>
>>> wrote:
>>>
>>>> On 04/10/18 22:07, isapir@apache.org wrote:
>>>>> Author: isapir Date: Thu Oct 4 21:07:54 2018 New Revision:
>>>>> 1842849
>>>>>
>>>>> URL: http://svn.apache.org/viewvc?rev=1842849&view=rev Log:
>>>>> System.load() expects absolute path.
>>>>
>>>> Remember to consider whether or not any changes you make to
>>>> trunk should be back-ported to 8.5.x and 7.0.x. Generally,
>>>> changes are back-ported unless they require changing a public
>>>> API (as defined in RELEASE-NOTES) or are considering likely to
>>>> cause a regression.
>>>>
>>>> <snip/>
>>>>
>>>>> Modified:
>>>> tomcat/trunk/test/org/apache/tomcat/util/net/openssl/TestOpenSSLConf
> .java
>>>>>
>>>>
> URL:
>>>> http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/tomcat/uti
> l/net/openssl/TestOpenSSLConf.java?rev=1842849&r1=1842848&r2=1842849&vie
> w=diff
>>>>>
>>>>
>>>>
> ========================================================================
> ======
>>>>> ---
>>>> tomcat/trunk/test/org/apache/tomcat/util/net/openssl/TestOpenSSLConf
> .java
>>>>
>>>>
> (original)
>>>>> +++
>>>> tomcat/trunk/test/org/apache/tomcat/util/net/openssl/TestOpenSSLConf
> .java
>>>>
>>>>
> Thu Oct 4 21:07:54 2018
>>>>> @@ -87,7 +87,11 @@ public class TestOpenSSLConf extends Tom
>>>>>
>>>>> @Test public void testOpenSSLConfCmdCipher() throws Exception
>>>>> { - log.info("Found OpenSSL version 0x" +
>>>> Integer.toHexString(OPENSSL_VERSION));
>>>>> + if (TesterSupport.isOpensslAvailable()) +
>>>>> log.info("Found OpenSSL version 0x" +
>>>> Integer.toHexString(OPENSSL_VERSION));
>>>>> + else + log.warn("OpenSSL not found: " +
>>>> TesterSupport.OPENSSL_ERROR);
>>>>> +
>>>>
>>>> The Tomcat style is to always use { and } even for one line for
>>>> clarity.
>>>>
>>>> Due to the age of the code base, there are a mix of styles.
>>>> Generally, we try and move code towards the currently accepted
>>>> style as we change it.
>>>>
>>>
>>> +1 a lack of { } is too big a possible bug source to ignore.
>
>> I just tried enabling the CheckStyle test for this. There were
>> just under three thousand errors.
>
>> I'm wondering if it is worth going through the code base fixing
>> these.
>
> I'm nearly -1 on this, mostly because it will make back-porting stuff
> a total PITA.
Fair enough. I don't need much convincing not to do it as I have plenty
of other stuff on my TODO list.
> Definitely opportunistically "upgrade" code we find here and there,
> but I don't think it's worth taking a day or two to add missing
> explicit blocks everywhere.
ACK.
>> On a related topic, I did notice several instance of the
>> following:
>
>> if (a == b) ... if (a == c) ... if (a == d) ...
>
>> that could be more efficiently written as:
>
>> if (a == b) { ... } else if (a == c) { ... } else if (a == d) {
>> ... }
>
> That would be nice. Sounds like a BZ issue that could have a
> "beginner" keyword attached.
Good idea. Feel free to add that if I don't get there first.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1842849 - in /tomcat/trunk:
java/org/apache/tomcat/jni/Library.java
test/org/apache/tomcat/util/net/TesterSupport.java
test/org/apache/tomcat/util/net/openssl/TestOpenSSLConf.java
Posted by Christopher Schultz <ch...@christopherschultz.net>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
Mark,
On 10/5/18 06:22, Mark Thomas wrote:
> On 05/10/18 10:42, Rémy Maucherat wrote:
>> On Fri, Oct 5, 2018 at 11:40 AM Mark Thomas <ma...@apache.org>
>> wrote:
>>
>>> On 04/10/18 22:07, isapir@apache.org wrote:
>>>> Author: isapir Date: Thu Oct 4 21:07:54 2018 New Revision:
>>>> 1842849
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=1842849&view=rev Log:
>>>> System.load() expects absolute path.
>>>
>>> Remember to consider whether or not any changes you make to
>>> trunk should be back-ported to 8.5.x and 7.0.x. Generally,
>>> changes are back-ported unless they require changing a public
>>> API (as defined in RELEASE-NOTES) or are considering likely to
>>> cause a regression.
>>>
>>> <snip/>
>>>
>>>> Modified:
>>> tomcat/trunk/test/org/apache/tomcat/util/net/openssl/TestOpenSSLConf
.java
>>>>
>>>
URL:
>>> http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/tomcat/uti
l/net/openssl/TestOpenSSLConf.java?rev=1842849&r1=1842848&r2=1842849&vie
w=diff
>>>>
>>>
>>>
========================================================================
======
>>>> ---
>>> tomcat/trunk/test/org/apache/tomcat/util/net/openssl/TestOpenSSLConf
.java
>>>
>>>
(original)
>>>> +++
>>> tomcat/trunk/test/org/apache/tomcat/util/net/openssl/TestOpenSSLConf
.java
>>>
>>>
Thu Oct 4 21:07:54 2018
>>>> @@ -87,7 +87,11 @@ public class TestOpenSSLConf extends Tom
>>>>
>>>> @Test public void testOpenSSLConfCmdCipher() throws Exception
>>>> { - log.info("Found OpenSSL version 0x" +
>>> Integer.toHexString(OPENSSL_VERSION));
>>>> + if (TesterSupport.isOpensslAvailable()) +
>>>> log.info("Found OpenSSL version 0x" +
>>> Integer.toHexString(OPENSSL_VERSION));
>>>> + else + log.warn("OpenSSL not found: " +
>>> TesterSupport.OPENSSL_ERROR);
>>>> +
>>>
>>> The Tomcat style is to always use { and } even for one line for
>>> clarity.
>>>
>>> Due to the age of the code base, there are a mix of styles.
>>> Generally, we try and move code towards the currently accepted
>>> style as we change it.
>>>
>>
>> +1 a lack of { } is too big a possible bug source to ignore.
>
> I just tried enabling the CheckStyle test for this. There were
> just under three thousand errors.
>
> I'm wondering if it is worth going through the code base fixing
> these.
I'm nearly -1 on this, mostly because it will make back-porting stuff
a total PITA.
Definitely opportunistically "upgrade" code we find here and there,
but I don't think it's worth taking a day or two to add missing
explicit blocks everywhere.
> On a related topic, I did notice several instance of the
> following:
>
> if (a == b) ... if (a == c) ... if (a == d) ...
>
> that could be more efficiently written as:
>
> if (a == b) { ... } else if (a == c) { ... } else if (a == d) {
> ... }
That would be nice. Sounds like a BZ issue that could have a
"beginner" keyword attached.
- -chris
-----BEGIN PGP SIGNATURE-----
Comment: Using GnuPG with Thunderbird - https://www.enigmail.net/
iQIzBAEBCAAdFiEEMmKgYcQvxMe7tcJcHPApP6U8pFgFAlu3sZoACgkQHPApP6U8
pFhg+BAAo5MYG+MA5ZXjEnhBg7/dMD2lyH7IhAXsL6DzTCtqd376JU12ZHChSfrT
cokAiDQT1TsNH4MsqleCu3kmrS9LuCKMMbVKNjVrR66CQKHnc6C/7hnw2blRLNUt
2NlOoelkT0biWWs81GmAOtMcESRFGlhKmypqtWbB4xeMFR8pPBAKu5MVCsfQRIX8
PleifXJO+6j7mTFEHHjbuc0/MuJT4taY/AErBoYCtqRwSJE7KZMqectAzBQNA6n/
ehAI7EDkGEupIvPxiMIh4I596tEwTiRnTZMX2CBpBj+YHHhXDUlRKyKu3R53964E
FSIPQqqmmfFoWBV3L5swaULkgi8/HxF2OAFeUqHQ7nSWURF32JgA4qZGU2d7Ot5P
o6EP2weM+CraFsH5lSncrY7EMJ7QUmtu5CsWOyTqLrw5A/KjCsscqz8RaY05b/AU
tKEDV6U6eQR0ka8Myn5faKDxXQMdkw9DdrydN1Wz8o0oMnKYaWXqjzKt6Ey8LpZm
sE4tw69Ji2D7Mu4WcSf+rqFCW9QaxEzDsu7Xv5CBMF9mRqjKC8bNIHk9wIyYn2JQ
GYULNRNMpKSKpYXvO0o1UPiMMTHYB0FpONH+jKb5Qlr6SRL18MjT8xY8R4A2vXGt
kcCHcFvWBUG87cZvmfzkSIkaFzocgqTRdW5Lmi4hSWN0pJCq7+Y=
=trg2
-----END PGP SIGNATURE-----
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1842849 - in /tomcat/trunk:
java/org/apache/tomcat/jni/Library.java
test/org/apache/tomcat/util/net/TesterSupport.java
test/org/apache/tomcat/util/net/openssl/TestOpenSSLConf.java
Posted by Mark Thomas <ma...@apache.org>.
On 05/10/18 10:42, Rémy Maucherat wrote:
> On Fri, Oct 5, 2018 at 11:40 AM Mark Thomas <ma...@apache.org> wrote:
>
>> On 04/10/18 22:07, isapir@apache.org wrote:
>>> Author: isapir
>>> Date: Thu Oct 4 21:07:54 2018
>>> New Revision: 1842849
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1842849&view=rev
>>> Log:
>>> System.load() expects absolute path.
>>
>> Remember to consider whether or not any changes you make to trunk should
>> be back-ported to 8.5.x and 7.0.x. Generally, changes are back-ported
>> unless they require changing a public API (as defined in RELEASE-NOTES)
>> or are considering likely to cause a regression.
>>
>> <snip/>
>>
>>> Modified:
>> tomcat/trunk/test/org/apache/tomcat/util/net/openssl/TestOpenSSLConf.java
>>> URL:
>> http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/tomcat/util/net/openssl/TestOpenSSLConf.java?rev=1842849&r1=1842848&r2=1842849&view=diff
>>>
>> ==============================================================================
>>> ---
>> tomcat/trunk/test/org/apache/tomcat/util/net/openssl/TestOpenSSLConf.java
>> (original)
>>> +++
>> tomcat/trunk/test/org/apache/tomcat/util/net/openssl/TestOpenSSLConf.java
>> Thu Oct 4 21:07:54 2018
>>> @@ -87,7 +87,11 @@ public class TestOpenSSLConf extends Tom
>>>
>>> @Test
>>> public void testOpenSSLConfCmdCipher() throws Exception {
>>> - log.info("Found OpenSSL version 0x" +
>> Integer.toHexString(OPENSSL_VERSION));
>>> + if (TesterSupport.isOpensslAvailable())
>>> + log.info("Found OpenSSL version 0x" +
>> Integer.toHexString(OPENSSL_VERSION));
>>> + else
>>> + log.warn("OpenSSL not found: " +
>> TesterSupport.OPENSSL_ERROR);
>>> +
>>
>> The Tomcat style is to always use { and } even for one line for clarity.
>>
>> Due to the age of the code base, there are a mix of styles. Generally,
>> we try and move code towards the currently accepted style as we change it.
>>
>
> +1 a lack of { } is too big a possible bug source to ignore.
I just tried enabling the CheckStyle test for this. There were just
under three thousand errors.
I'm wondering if it is worth going through the code base fixing these.
On a related topic, I did notice several instance of the following:
if (a == b) ...
if (a == c) ...
if (a == d) ...
that could be more efficiently written as:
if (a == b) {
...
} else if (a == c) {
...
} else if (a == d) {
...
}
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1842849 - in /tomcat/trunk: java/org/apache/tomcat/jni/Library.java
test/org/apache/tomcat/util/net/TesterSupport.java test/org/apache/tomcat/util/net/openssl/TestOpenSSLConf.java
Posted by Rémy Maucherat <re...@apache.org>.
On Fri, Oct 5, 2018 at 11:40 AM Mark Thomas <ma...@apache.org> wrote:
> On 04/10/18 22:07, isapir@apache.org wrote:
> > Author: isapir
> > Date: Thu Oct 4 21:07:54 2018
> > New Revision: 1842849
> >
> > URL: http://svn.apache.org/viewvc?rev=1842849&view=rev
> > Log:
> > System.load() expects absolute path.
>
> Remember to consider whether or not any changes you make to trunk should
> be back-ported to 8.5.x and 7.0.x. Generally, changes are back-ported
> unless they require changing a public API (as defined in RELEASE-NOTES)
> or are considering likely to cause a regression.
>
> <snip/>
>
> > Modified:
> tomcat/trunk/test/org/apache/tomcat/util/net/openssl/TestOpenSSLConf.java
> > URL:
> http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/tomcat/util/net/openssl/TestOpenSSLConf.java?rev=1842849&r1=1842848&r2=1842849&view=diff
> >
> ==============================================================================
> > ---
> tomcat/trunk/test/org/apache/tomcat/util/net/openssl/TestOpenSSLConf.java
> (original)
> > +++
> tomcat/trunk/test/org/apache/tomcat/util/net/openssl/TestOpenSSLConf.java
> Thu Oct 4 21:07:54 2018
> > @@ -87,7 +87,11 @@ public class TestOpenSSLConf extends Tom
> >
> > @Test
> > public void testOpenSSLConfCmdCipher() throws Exception {
> > - log.info("Found OpenSSL version 0x" +
> Integer.toHexString(OPENSSL_VERSION));
> > + if (TesterSupport.isOpensslAvailable())
> > + log.info("Found OpenSSL version 0x" +
> Integer.toHexString(OPENSSL_VERSION));
> > + else
> > + log.warn("OpenSSL not found: " +
> TesterSupport.OPENSSL_ERROR);
> > +
>
> The Tomcat style is to always use { and } even for one line for clarity.
>
> Due to the age of the code base, there are a mix of styles. Generally,
> we try and move code towards the currently accepted style as we change it.
>
+1 a lack of { } is too big a possible bug source to ignore.
Rémy
Re: svn commit: r1842849 - in /tomcat/trunk:
java/org/apache/tomcat/jni/Library.java
test/org/apache/tomcat/util/net/TesterSupport.java
test/org/apache/tomcat/util/net/openssl/TestOpenSSLConf.java
Posted by Igal Sapir <is...@apache.org>.
On 10/5/2018 2:40 AM, Mark Thomas wrote:
> On 04/10/18 22:07, isapir@apache.org wrote:
>> Author: isapir
>> Date: Thu Oct 4 21:07:54 2018
>> New Revision: 1842849
>>
>> URL: http://svn.apache.org/viewvc?rev=1842849&view=rev
>> Log:
>> System.load() expects absolute path.
> Remember to consider whether or not any changes you make to trunk should
> be back-ported to 8.5.x and 7.0.x. Generally, changes are back-ported
> unless they require changing a public API (as defined in RELEASE-NOTES)
> or are considering likely to cause a regression.
Got it. I will set up the other branches locally. So far working
mostly with trunk.
> <snip/>
>
>> Modified: tomcat/trunk/test/org/apache/tomcat/util/net/openssl/TestOpenSSLConf.java
>> URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/tomcat/util/net/openssl/TestOpenSSLConf.java?rev=1842849&r1=1842848&r2=1842849&view=diff
>> ==============================================================================
>> --- tomcat/trunk/test/org/apache/tomcat/util/net/openssl/TestOpenSSLConf.java (original)
>> +++ tomcat/trunk/test/org/apache/tomcat/util/net/openssl/TestOpenSSLConf.java Thu Oct 4 21:07:54 2018
>> @@ -87,7 +87,11 @@ public class TestOpenSSLConf extends Tom
>>
>> @Test
>> public void testOpenSSLConfCmdCipher() throws Exception {
>> - log.info("Found OpenSSL version 0x" + Integer.toHexString(OPENSSL_VERSION));
>> + if (TesterSupport.isOpensslAvailable())
>> + log.info("Found OpenSSL version 0x" + Integer.toHexString(OPENSSL_VERSION));
>> + else
>> + log.warn("OpenSSL not found: " + TesterSupport.OPENSSL_ERROR);
>> +
> The Tomcat style is to always use { and } even for one line for clarity.
>
> Due to the age of the code base, there are a mix of styles. Generally,
> we try and move code towards the currently accepted style as we change it.
Understood. I usually try to look at existing code to understand the
style and that mix always threw me off. Thanks for clarifying.
Igal
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1842849 - in /tomcat/trunk:
java/org/apache/tomcat/jni/Library.java
test/org/apache/tomcat/util/net/TesterSupport.java
test/org/apache/tomcat/util/net/openssl/TestOpenSSLConf.java
Posted by Mark Thomas <ma...@apache.org>.
On 04/10/18 22:07, isapir@apache.org wrote:
> Author: isapir
> Date: Thu Oct 4 21:07:54 2018
> New Revision: 1842849
>
> URL: http://svn.apache.org/viewvc?rev=1842849&view=rev
> Log:
> System.load() expects absolute path.
Remember to consider whether or not any changes you make to trunk should
be back-ported to 8.5.x and 7.0.x. Generally, changes are back-ported
unless they require changing a public API (as defined in RELEASE-NOTES)
or are considering likely to cause a regression.
<snip/>
> Modified: tomcat/trunk/test/org/apache/tomcat/util/net/openssl/TestOpenSSLConf.java
> URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/tomcat/util/net/openssl/TestOpenSSLConf.java?rev=1842849&r1=1842848&r2=1842849&view=diff
> ==============================================================================
> --- tomcat/trunk/test/org/apache/tomcat/util/net/openssl/TestOpenSSLConf.java (original)
> +++ tomcat/trunk/test/org/apache/tomcat/util/net/openssl/TestOpenSSLConf.java Thu Oct 4 21:07:54 2018
> @@ -87,7 +87,11 @@ public class TestOpenSSLConf extends Tom
>
> @Test
> public void testOpenSSLConfCmdCipher() throws Exception {
> - log.info("Found OpenSSL version 0x" + Integer.toHexString(OPENSSL_VERSION));
> + if (TesterSupport.isOpensslAvailable())
> + log.info("Found OpenSSL version 0x" + Integer.toHexString(OPENSSL_VERSION));
> + else
> + log.warn("OpenSSL not found: " + TesterSupport.OPENSSL_ERROR);
> +
The Tomcat style is to always use { and } even for one line for clarity.
Due to the age of the code base, there are a mix of styles. Generally,
we try and move code towards the currently accepted style as we change it.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org