You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by rp...@apache.org on 2015/08/20 18:13:27 UTC
logging-log4j2 git commit: LOG4J2-1096 bugfix and additional unit
test for case where one or more arguments is null
Repository: logging-log4j2
Updated Branches:
refs/heads/master 8124d9b0d -> 31b7e7be5
LOG4J2-1096 bugfix and additional unit test for case where one or more
arguments is null
Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo
Commit: http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/31b7e7be
Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/31b7e7be
Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/31b7e7be
Branch: refs/heads/master
Commit: 31b7e7be51b2509680fe6e01c1ede99426ee81a0
Parents: 8124d9b
Author: rpopma <rp...@apache.org>
Authored: Fri Aug 21 01:13:37 2015 +0900
Committer: rpopma <rp...@apache.org>
Committed: Fri Aug 21 01:13:37 2015 +0900
----------------------------------------------------------------------
.../logging/log4j/message/ParameterizedMessage.java | 10 +++++-----
.../logging/log4j/message/ParameterizedMessageTest.java | 8 ++++++++
2 files changed, 13 insertions(+), 5 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/31b7e7be/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterizedMessage.java
----------------------------------------------------------------------
diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterizedMessage.java b/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterizedMessage.java
index 1a9175b..e5891c4 100644
--- a/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterizedMessage.java
+++ b/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterizedMessage.java
@@ -323,11 +323,11 @@ public class ParameterizedMessage implements Message {
/**
* Returns the sum of the lengths of all Strings in the specified array.
*/
- // 27 bytes
+ // 30 bytes
private static int sumStringLengths(final String[] arguments) {
int result = 0;
for (int i = 0; i < arguments.length; i++) {
- result += arguments[i].length();
+ result += String.valueOf(arguments[i]).length();
}
return result;
}
@@ -441,11 +441,11 @@ public class ParameterizedMessage implements Message {
* Appends the argument at the specified argument index to the specified result char array at the specified position
* and returns the resulting position.
*/
- // 27 bytes
+ // 30 bytes
private static int writeArgAt0(final String[] arguments, final int currentArgument, final char[] result,
final int pos) {
- final String arg = arguments[currentArgument];
- final int argLen = arg.length();
+ final String arg = String.valueOf(arguments[currentArgument]);
+ int argLen = arg.length();
arg.getChars(0, argLen, result, pos);
return pos + argLen;
}
http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/31b7e7be/log4j-api/src/test/java/org/apache/logging/log4j/message/ParameterizedMessageTest.java
----------------------------------------------------------------------
diff --git a/log4j-api/src/test/java/org/apache/logging/log4j/message/ParameterizedMessageTest.java b/log4j-api/src/test/java/org/apache/logging/log4j/message/ParameterizedMessageTest.java
index 5ca48df..d4ba0de 100644
--- a/log4j-api/src/test/java/org/apache/logging/log4j/message/ParameterizedMessageTest.java
+++ b/log4j-api/src/test/java/org/apache/logging/log4j/message/ParameterizedMessageTest.java
@@ -46,6 +46,14 @@ public class ParameterizedMessageTest {
}
@Test
+ public void testFormatNullArgs() {
+ final String testMsg = "Test message {} {} {} {} {} {}";
+ final String[] args = { "a", null, "c", null, null, null };
+ final String result = ParameterizedMessage.formatStringArgs(testMsg, args);
+ assertEquals("Test message a null c null null null", result);
+ }
+
+ @Test
public void testFormatStringArgsIgnoresSuperfluousArgs() {
final String testMsg = "Test message {}{} {}";
final String[] args = { "a", "b", "c", "unnecessary", "superfluous" };
Re: logging-log4j2 git commit: LOG4J2-1096 bugfix and additional unit
test for case where one or more arguments is null
Posted by Remko Popma <re...@gmail.com>.
I agree. I made some changes clarifying that (a) this is a performance
sensitive method, and (b) why the method size is important and (c) linked
to the Jira containing the performance measurements.
Note that I don't intend to do this with all methods. :-)
Only places where profiling shows that log4j is spending most of its time.
However, for such methods it is good to have such marker comments to
prevent them from being casually modified.
On Fri, Aug 21, 2015 at 1:38 AM, Gary Gregory <ga...@gmail.com>
wrote:
> I think we need better comments than:
>
> // 30 bytes
>
> Think of what someone checking out the code would think and then do to
> modify the code.
>
> At the very minimum a URL to the Jira issue is required.
>
> Gary
> ---------- Forwarded message ----------
> From: <rp...@apache.org>
> Date: Thu, Aug 20, 2015 at 9:13 AM
> Subject: logging-log4j2 git commit: LOG4J2-1096 bugfix and additional unit
> test for case where one or more arguments is null
> To: commits@logging.apache.org
>
>
> Repository: logging-log4j2
> Updated Branches:
> refs/heads/master 8124d9b0d -> 31b7e7be5
>
>
> LOG4J2-1096 bugfix and additional unit test for case where one or more
> arguments is null
>
> Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo
> Commit:
> http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/31b7e7be
> Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/31b7e7be
> Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/31b7e7be
>
> Branch: refs/heads/master
> Commit: 31b7e7be51b2509680fe6e01c1ede99426ee81a0
> Parents: 8124d9b
> Author: rpopma <rp...@apache.org>
> Authored: Fri Aug 21 01:13:37 2015 +0900
> Committer: rpopma <rp...@apache.org>
> Committed: Fri Aug 21 01:13:37 2015 +0900
>
> ----------------------------------------------------------------------
> .../logging/log4j/message/ParameterizedMessage.java | 10 +++++-----
> .../logging/log4j/message/ParameterizedMessageTest.java | 8 ++++++++
> 2 files changed, 13 insertions(+), 5 deletions(-)
> ----------------------------------------------------------------------
>
>
>
> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/31b7e7be/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterizedMessage.java
> ----------------------------------------------------------------------
> diff --git
> a/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterizedMessage.java
> b/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterizedMessage.java
> index 1a9175b..e5891c4 100644
> ---
> a/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterizedMessage.java
> +++
> b/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterizedMessage.java
> @@ -323,11 +323,11 @@ public class ParameterizedMessage implements Message
> {
> /**
> * Returns the sum of the lengths of all Strings in the specified
> array.
> */
> - // 27 bytes
> + // 30 bytes
> private static int sumStringLengths(final String[] arguments) {
> int result = 0;
> for (int i = 0; i < arguments.length; i++) {
> - result += arguments[i].length();
> + result += String.valueOf(arguments[i]).length();
> }
> return result;
> }
> @@ -441,11 +441,11 @@ public class ParameterizedMessage implements Message
> {
> * Appends the argument at the specified argument index to the
> specified result char array at the specified position
> * and returns the resulting position.
> */
> - // 27 bytes
> + // 30 bytes
> private static int writeArgAt0(final String[] arguments, final int
> currentArgument, final char[] result,
> final int pos) {
> - final String arg = arguments[currentArgument];
> - final int argLen = arg.length();
> + final String arg = String.valueOf(arguments[currentArgument]);
> + int argLen = arg.length();
> arg.getChars(0, argLen, result, pos);
> return pos + argLen;
> }
>
>
> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/31b7e7be/log4j-api/src/test/java/org/apache/logging/log4j/message/ParameterizedMessageTest.java
> ----------------------------------------------------------------------
> diff --git
> a/log4j-api/src/test/java/org/apache/logging/log4j/message/ParameterizedMessageTest.java
> b/log4j-api/src/test/java/org/apache/logging/log4j/message/ParameterizedMessageTest.java
> index 5ca48df..d4ba0de 100644
> ---
> a/log4j-api/src/test/java/org/apache/logging/log4j/message/ParameterizedMessageTest.java
> +++
> b/log4j-api/src/test/java/org/apache/logging/log4j/message/ParameterizedMessageTest.java
> @@ -46,6 +46,14 @@ public class ParameterizedMessageTest {
> }
>
> @Test
> + public void testFormatNullArgs() {
> + final String testMsg = "Test message {} {} {} {} {} {}";
> + final String[] args = { "a", null, "c", null, null, null };
> + final String result =
> ParameterizedMessage.formatStringArgs(testMsg, args);
> + assertEquals("Test message a null c null null null", result);
> + }
> +
> + @Test
> public void testFormatStringArgsIgnoresSuperfluousArgs() {
> final String testMsg = "Test message {}{} {}";
> final String[] args = { "a", "b", "c", "unnecessary",
> "superfluous" };
>
>
>
>
> --
> E-Mail: garydgregory@gmail.com | ggregory@apache.org
> Java Persistence with Hibernate, Second Edition
> <http://www.manning.com/bauer3/>
> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
> Spring Batch in Action <http://www.manning.com/templier/>
> Blog: http://garygregory.wordpress.com
> Home: http://garygregory.com/
> Tweet! http://twitter.com/GaryGregory
>
Fwd: logging-log4j2 git commit: LOG4J2-1096 bugfix and additional
unit test for case where one or more arguments is null
Posted by Gary Gregory <ga...@gmail.com>.
I think we need better comments than:
// 30 bytes
Think of what someone checking out the code would think and then do to
modify the code.
At the very minimum a URL to the Jira issue is required.
Gary
---------- Forwarded message ----------
From: <rp...@apache.org>
Date: Thu, Aug 20, 2015 at 9:13 AM
Subject: logging-log4j2 git commit: LOG4J2-1096 bugfix and additional unit
test for case where one or more arguments is null
To: commits@logging.apache.org
Repository: logging-log4j2
Updated Branches:
refs/heads/master 8124d9b0d -> 31b7e7be5
LOG4J2-1096 bugfix and additional unit test for case where one or more
arguments is null
Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo
Commit:
http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/31b7e7be
Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/31b7e7be
Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/31b7e7be
Branch: refs/heads/master
Commit: 31b7e7be51b2509680fe6e01c1ede99426ee81a0
Parents: 8124d9b
Author: rpopma <rp...@apache.org>
Authored: Fri Aug 21 01:13:37 2015 +0900
Committer: rpopma <rp...@apache.org>
Committed: Fri Aug 21 01:13:37 2015 +0900
----------------------------------------------------------------------
.../logging/log4j/message/ParameterizedMessage.java | 10 +++++-----
.../logging/log4j/message/ParameterizedMessageTest.java | 8 ++++++++
2 files changed, 13 insertions(+), 5 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/31b7e7be/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterizedMessage.java
----------------------------------------------------------------------
diff --git
a/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterizedMessage.java
b/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterizedMessage.java
index 1a9175b..e5891c4 100644
---
a/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterizedMessage.java
+++
b/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterizedMessage.java
@@ -323,11 +323,11 @@ public class ParameterizedMessage implements Message {
/**
* Returns the sum of the lengths of all Strings in the specified
array.
*/
- // 27 bytes
+ // 30 bytes
private static int sumStringLengths(final String[] arguments) {
int result = 0;
for (int i = 0; i < arguments.length; i++) {
- result += arguments[i].length();
+ result += String.valueOf(arguments[i]).length();
}
return result;
}
@@ -441,11 +441,11 @@ public class ParameterizedMessage implements Message {
* Appends the argument at the specified argument index to the
specified result char array at the specified position
* and returns the resulting position.
*/
- // 27 bytes
+ // 30 bytes
private static int writeArgAt0(final String[] arguments, final int
currentArgument, final char[] result,
final int pos) {
- final String arg = arguments[currentArgument];
- final int argLen = arg.length();
+ final String arg = String.valueOf(arguments[currentArgument]);
+ int argLen = arg.length();
arg.getChars(0, argLen, result, pos);
return pos + argLen;
}
http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/31b7e7be/log4j-api/src/test/java/org/apache/logging/log4j/message/ParameterizedMessageTest.java
----------------------------------------------------------------------
diff --git
a/log4j-api/src/test/java/org/apache/logging/log4j/message/ParameterizedMessageTest.java
b/log4j-api/src/test/java/org/apache/logging/log4j/message/ParameterizedMessageTest.java
index 5ca48df..d4ba0de 100644
---
a/log4j-api/src/test/java/org/apache/logging/log4j/message/ParameterizedMessageTest.java
+++
b/log4j-api/src/test/java/org/apache/logging/log4j/message/ParameterizedMessageTest.java
@@ -46,6 +46,14 @@ public class ParameterizedMessageTest {
}
@Test
+ public void testFormatNullArgs() {
+ final String testMsg = "Test message {} {} {} {} {} {}";
+ final String[] args = { "a", null, "c", null, null, null };
+ final String result =
ParameterizedMessage.formatStringArgs(testMsg, args);
+ assertEquals("Test message a null c null null null", result);
+ }
+
+ @Test
public void testFormatStringArgsIgnoresSuperfluousArgs() {
final String testMsg = "Test message {}{} {}";
final String[] args = { "a", "b", "c", "unnecessary",
"superfluous" };
--
E-Mail: garydgregory@gmail.com | ggregory@apache.org
Java Persistence with Hibernate, Second Edition
<http://www.manning.com/bauer3/>
JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
Spring Batch in Action <http://www.manning.com/templier/>
Blog: http://garygregory.wordpress.com
Home: http://garygregory.com/
Tweet! http://twitter.com/GaryGregory