I think using Objects.requireNonNull() is more precise FWIW. Gary
On Sun, Dec 25, 2022, 16:38 <pkarw...@apache.org> wrote: > This is an automated email from the ASF dual-hosted git repository. > > pkarwasz pushed a commit to branch master > in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git > > > The following commit(s) were added to refs/heads/master by this push: > new ffc82a4820 Fix NPE in Log4jMarker > ffc82a4820 is described below > > commit ffc82a48203fb890d85e3a497476611368ba1021 > Author: Piotr P. Karwasz <piotr.git...@karwasz.org> > AuthorDate: Sun Dec 25 22:30:27 2022 +0100 > > Fix NPE in Log4jMarker > > Log4j2's Marker#getParents() does actually return `null` instead of an > empty array. > --- > .../java/org/apache/logging/slf4j/Log4jMarker.java | 83 > ++++++++++------------ > .../java/org/apache/logging/slf4j/Log4jMarker.java | 9 +-- > 2 files changed, 44 insertions(+), 48 deletions(-) > > diff --git > a/log4j-slf4j-impl/src/main/java/org/apache/logging/slf4j/Log4jMarker.java > b/log4j-slf4j-impl/src/main/java/org/apache/logging/slf4j/Log4jMarker.java > index 30931851f0..bb1621aa39 100644 > --- > a/log4j-slf4j-impl/src/main/java/org/apache/logging/slf4j/Log4jMarker.java > +++ > b/log4j-slf4j-impl/src/main/java/org/apache/logging/slf4j/Log4jMarker.java > @@ -17,8 +17,10 @@ > package org.apache.logging.slf4j; > > import java.util.ArrayList; > +import java.util.Collections; > import java.util.Iterator; > import java.util.List; > +import java.util.Objects; > > import org.apache.logging.log4j.MarkerManager; > import org.slf4j.IMarkerFactory; > @@ -46,47 +48,40 @@ class Log4jMarker implements Marker { > > @Override > public void add(final Marker marker) { > - if (marker == null) { > - throw new IllegalArgumentException(); > - } > + if (marker == null) { > + throw new IllegalArgumentException(); > + } > final Marker m = factory.getMarker(marker.getName()); > this.marker.addParents(((Log4jMarker)m).getLog4jMarker()); > } > > @Override > - public boolean contains(final Marker marker) { > - if (marker == null) { > - throw new IllegalArgumentException(); > - } > - return this.marker.isInstanceOf(marker.getName()); > - } > + public boolean contains(final Marker marker) { > + if (marker == null) { > + throw new IllegalArgumentException(); > + } > + return this.marker.isInstanceOf(marker.getName()); > + } > > @Override > - public boolean contains(final String s) { > - return s != null && this.marker.isInstanceOf(s); > - } > + public boolean contains(final String s) { > + return s != null ? this.marker.isInstanceOf(s) : false; > + } > > @Override > - public boolean equals(final Object obj) { > - if (this == obj) { > - return true; > - } > - if (obj == null) { > - return false; > - } > - if (!(obj instanceof Log4jMarker)) { > - return false; > - } > - final Log4jMarker other = (Log4jMarker) obj; > - if (marker == null) { > - if (other.marker != null) { > - return false; > - } > - } else if (!marker.equals(other.marker)) { > - return false; > - } > - return true; > - } > + public boolean equals(final Object obj) { > + if (this == obj) { > + return true; > + } > + if (obj == null) { > + return false; > + } > + if (!(obj instanceof Log4jMarker)) { > + return false; > + } > + final Log4jMarker other = (Log4jMarker) obj; > + return Objects.equals(marker, other.marker); > + } > > public org.apache.logging.log4j.Marker getLog4jMarker() { > return marker; > @@ -103,30 +98,30 @@ class Log4jMarker implements Marker { > } > > @Override > - public int hashCode() { > - final int prime = 31; > - int result = 1; > - result = prime * result + ((marker == null) ? 0 : > marker.hashCode()); > - return result; > - } > + public int hashCode() { > + return 31 + Objects.hashCode(marker); > + } > > @Override > public boolean hasReferences() { > return marker.hasParents(); > } > > - @Override > + @Override > public Iterator<Marker> iterator() { > final org.apache.logging.log4j.Marker[] log4jParents = > this.marker.getParents(); > + if (log4jParents == null) { > + return Collections.emptyIterator(); > + } > final List<Marker> parents = new ArrayList<>(log4jParents.length); > - for (final org.apache.logging.log4j.Marker m : > log4jParents) { > + for (final org.apache.logging.log4j.Marker m : log4jParents) { > parents.add(factory.getMarker(m.getName())); > } > return parents.iterator(); > } > > - @Override > - public boolean remove(final Marker marker) { > - return marker != null && > this.marker.remove(MarkerManager.getMarker(marker.getName())); > - } > + @Override > + public boolean remove(final Marker marker) { > + return marker != null ? > this.marker.remove(MarkerManager.getMarker(marker.getName())) : false; > + } > } > diff --git > a/log4j-slf4j2-impl/src/main/java/org/apache/logging/slf4j/Log4jMarker.java > b/log4j-slf4j2-impl/src/main/java/org/apache/logging/slf4j/Log4jMarker.java > index 1b96e1a010..bb1621aa39 100644 > --- > a/log4j-slf4j2-impl/src/main/java/org/apache/logging/slf4j/Log4jMarker.java > +++ > b/log4j-slf4j2-impl/src/main/java/org/apache/logging/slf4j/Log4jMarker.java > @@ -17,6 +17,7 @@ > package org.apache.logging.slf4j; > > import java.util.ArrayList; > +import java.util.Collections; > import java.util.Iterator; > import java.util.List; > import java.util.Objects; > @@ -79,10 +80,7 @@ class Log4jMarker implements Marker { > return false; > } > final Log4jMarker other = (Log4jMarker) obj; > - if (!Objects.equals(marker, other.marker)) { > - return false; > - } > - return true; > + return Objects.equals(marker, other.marker); > } > > public org.apache.logging.log4j.Marker getLog4jMarker() { > @@ -112,6 +110,9 @@ class Log4jMarker implements Marker { > @Override > public Iterator<Marker> iterator() { > final org.apache.logging.log4j.Marker[] log4jParents = > this.marker.getParents(); > + if (log4jParents == null) { > + return Collections.emptyIterator(); > + } > final List<Marker> parents = new ArrayList<>(log4jParents.length); > for (final org.apache.logging.log4j.Marker m : log4jParents) { > parents.add(factory.getMarker(m.getName())); > >
