You are viewing a plain text version of this content. The canonical link for it is here.
Posted to log4j-dev@logging.apache.org by Remko Popma <re...@gmail.com> on 2014/09/19 03:01:13 UTC
Re: git commit: Refactor MAC address getter to own method.
On an enterprise machine it is not uncommon to have multiple network interface cards. When used internally in UUID, it is okay to ignore that scenario, and it's also more convenient to return localhost rather than null if there is no network card at all.
If we make this logic public, we cannot make the same assumptions any more. Shouldn't we return some sort of iterator or collection of MacAddresses? Or call it getFirstMacAddress? The Javadoc says null is returned if there's no NIC, but I don't see that in the code. (Do you anticipate using this method for something other than UUID? It might be better to leave it private...)
Just my 2c.
Remko
Sent from my iPhone
> On 2014/09/19, at 9:05, mattsicker@apache.org wrote:
>
> Repository: logging-log4j2
> Updated Branches:
> refs/heads/master 6b80804dc -> 2a754e12b
>
>
> Refactor MAC address getter to own method.
>
>
> Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo
> Commit: http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/2a754e12
> Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/2a754e12
> Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/2a754e12
>
> Branch: refs/heads/master
> Commit: 2a754e12b28df03dab38d5e66db251094a9834a9
> Parents: 6b80804
> Author: Matt Sicker <ma...@apache.org>
> Authored: Thu Sep 18 19:04:56 2014 -0500
> Committer: Matt Sicker <ma...@apache.org>
> Committed: Thu Sep 18 19:04:56 2014 -0500
>
> ----------------------------------------------------------------------
> .../logging/log4j/core/util/NetUtils.java | 40 ++++++++++++++++++++
> .../logging/log4j/core/util/UuidUtil.java | 40 +-------------------
> 2 files changed, 41 insertions(+), 39 deletions(-)
> ----------------------------------------------------------------------
>
>
> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/2a754e12/log4j-core/src/main/java/org/apache/logging/log4j/core/util/NetUtils.java
> ----------------------------------------------------------------------
> diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/NetUtils.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/NetUtils.java
> index 2609fef..46e16ce 100644
> --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/NetUtils.java
> +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/NetUtils.java
> @@ -70,4 +70,44 @@ public final class NetUtils {
> return "UNKNOWN_LOCALHOST";
> }
> }
> +
> + /**
> + * Returns the local network interface's MAC address if possible. The local network interface is defined here as
> + * the {@link NetworkInterface} that is both up and not a loopback interface.
> + *
> + * @return the MAC address of the local network interface or {@code null} if no MAC address could be determined.
> + * @since 2.1
> + */
> + public static byte[] getLocalMacAddress() {
> + byte[] mac = null;
> + try {
> + final InetAddress localHost = InetAddress.getLocalHost();
> + try {
> + final NetworkInterface localInterface = NetworkInterface.getByInetAddress(localHost);
> + if (isUpAndNotLoopback(localInterface)) {
> + mac = localInterface.getHardwareAddress();
> + }
> + if (mac == null) {
> + final Enumeration<NetworkInterface> networkInterfaces = NetworkInterface.getNetworkInterfaces();
> + while (networkInterfaces.hasMoreElements() && mac == null) {
> + final NetworkInterface nic = networkInterfaces.nextElement();
> + if (isUpAndNotLoopback(nic)) {
> + mac = nic.getHardwareAddress();
> + }
> + }
> + }
> + } catch (final SocketException e) {
> + LOGGER.catching(e);
> + }
> + if (mac == null || mac.length == 0) {
> + mac = localHost.getAddress();
> + }
> + } catch (final UnknownHostException ignored) {
> + }
> + return mac;
> + }
> +
> + private static boolean isUpAndNotLoopback(final NetworkInterface ni) throws SocketException {
> + return ni != null && !ni.isLoopback() && ni.isUp();
> + }
> }
>
> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/2a754e12/log4j-core/src/main/java/org/apache/logging/log4j/core/util/UuidUtil.java
> ----------------------------------------------------------------------
> diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/UuidUtil.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/UuidUtil.java
> index 34b021f..9bed752 100644
> --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/UuidUtil.java
> +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/UuidUtil.java
> @@ -16,13 +16,8 @@
> */
> package org.apache.logging.log4j.core.util;
>
> -import java.lang.reflect.Method;
> -import java.net.InetAddress;
> -import java.net.NetworkInterface;
> -import java.net.UnknownHostException;
> import java.nio.ByteBuffer;
> import java.security.SecureRandom;
> -import java.util.Enumeration;
> import java.util.Random;
> import java.util.UUID;
> import java.util.concurrent.atomic.AtomicInteger;
> @@ -65,40 +60,7 @@ public final class UuidUtil {
> private static final int HUNDRED_NANOS_PER_MILLI = 10000;
>
> static {
> - byte[] mac = null;
> - try {
> - final InetAddress address = InetAddress.getLocalHost();
> - try {
> - NetworkInterface ni = NetworkInterface.getByInetAddress(address);
> - if (ni != null && !ni.isLoopback() && ni.isUp()) {
> - final Method method = ni.getClass().getMethod("getHardwareAddress");
> - if (method != null) {
> - mac = (byte[]) method.invoke(ni);
> - }
> - }
> -
> - if (mac == null) {
> - final Enumeration<NetworkInterface> enumeration = NetworkInterface.getNetworkInterfaces();
> - while (enumeration.hasMoreElements() && mac == null) {
> - ni = enumeration.nextElement();
> - if (ni != null && ni.isUp() && !ni.isLoopback()) {
> - final Method method = ni.getClass().getMethod("getHardwareAddress");
> - if (method != null) {
> - mac = (byte[]) method.invoke(ni);
> - }
> - }
> - }
> - }
> - } catch (final Exception ex) {
> - ex.printStackTrace();
> - // Ignore exception
> - }
> - if (mac == null || mac.length == 0) {
> - mac = address.getAddress();
> - }
> - } catch (final UnknownHostException e) {
> - // Ignore exception
> - }
> + byte[] mac = NetUtils.getLocalMacAddress();
> final Random randomGenerator = new SecureRandom();
> if (mac == null || mac.length == 0) {
> mac = new byte[6];
>
---------------------------------------------------------------------
To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
For additional commands, e-mail: log4j-dev-help@logging.apache.org
Re: git commit: Refactor MAC address getter to own method.
Posted by Matt Sicker <bo...@gmail.com>.
The reason I had at the time was that it seemed like it made more sense in
NetUtils, but in general, it seemed like a nice idea to break down the
static initializer block into its logical functions. I can move it back
into UuidUtil and make it private.
On 18 September 2014 23:03, Ralph Goers <ra...@dslextreme.com> wrote:
> What was the reason for pulling it out of UUIDUtil in the first place? Is
> it going to be used somewhere else?
>
> Ralph
>
> On Sep 18, 2014, at 7:08 PM, Remko Popma <re...@gmail.com> wrote:
>
> If you anticipate using it in more places in this package, otherwise my
> preference would be to make it private, and increase the visibility when a
> new use case comes up.
>
> Sent from my iPhone
>
> On 2014/09/19, at 10:51, Matt Sicker <bo...@gmail.com> wrote:
>
> Package-private perhaps?
>
> On 18 September 2014 20:01, Remko Popma <re...@gmail.com> wrote:
>
>> On an enterprise machine it is not uncommon to have multiple network
>> interface cards. When used internally in UUID, it is okay to ignore that
>> scenario, and it's also more convenient to return localhost rather than
>> null if there is no network card at all.
>>
>> If we make this logic public, we cannot make the same assumptions any
>> more. Shouldn't we return some sort of iterator or collection of
>> MacAddresses? Or call it getFirstMacAddress? The Javadoc says null is
>> returned if there's no NIC, but I don't see that in the code. (Do you
>> anticipate using this method for something other than UUID? It might be
>> better to leave it private...)
>>
>> Just my 2c.
>> Remko
>>
>> Sent from my iPhone
>>
>> > On 2014/09/19, at 9:05, mattsicker@apache.org wrote:
>> >
>> > Repository: logging-log4j2
>> > Updated Branches:
>> > refs/heads/master 6b80804dc -> 2a754e12b
>> >
>> >
>> > Refactor MAC address getter to own method.
>> >
>> >
>> > Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo
>> > Commit:
>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/2a754e12
>> > Tree:
>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/2a754e12
>> > Diff:
>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/2a754e12
>> >
>> > Branch: refs/heads/master
>> > Commit: 2a754e12b28df03dab38d5e66db251094a9834a9
>> > Parents: 6b80804
>> > Author: Matt Sicker <ma...@apache.org>
>> > Authored: Thu Sep 18 19:04:56 2014 -0500
>> > Committer: Matt Sicker <ma...@apache.org>
>> > Committed: Thu Sep 18 19:04:56 2014 -0500
>> >
>> > ----------------------------------------------------------------------
>> > .../logging/log4j/core/util/NetUtils.java | 40
>> ++++++++++++++++++++
>> > .../logging/log4j/core/util/UuidUtil.java | 40
>> +-------------------
>> > 2 files changed, 41 insertions(+), 39 deletions(-)
>> > ----------------------------------------------------------------------
>> >
>> >
>> >
>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/2a754e12/log4j-core/src/main/java/org/apache/logging/log4j/core/util/NetUtils.java
>> > ----------------------------------------------------------------------
>> > diff --git
>> a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/NetUtils.java
>> b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/NetUtils.java
>> > index 2609fef..46e16ce 100644
>> > ---
>> a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/NetUtils.java
>> > +++
>> b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/NetUtils.java
>> > @@ -70,4 +70,44 @@ public final class NetUtils {
>> > return "UNKNOWN_LOCALHOST";
>> > }
>> > }
>> > +
>> > + /**
>> > + * Returns the local network interface's MAC address if possible.
>> The local network interface is defined here as
>> > + * the {@link NetworkInterface} that is both up and not a loopback
>> interface.
>> > + *
>> > + * @return the MAC address of the local network interface or
>> {@code null} if no MAC address could be determined.
>> > + * @since 2.1
>> > + */
>> > + public static byte[] getLocalMacAddress() {
>> > + byte[] mac = null;
>> > + try {
>> > + final InetAddress localHost = InetAddress.getLocalHost();
>> > + try {
>> > + final NetworkInterface localInterface =
>> NetworkInterface.getByInetAddress(localHost);
>> > + if (isUpAndNotLoopback(localInterface)) {
>> > + mac = localInterface.getHardwareAddress();
>> > + }
>> > + if (mac == null) {
>> > + final Enumeration<NetworkInterface>
>> networkInterfaces = NetworkInterface.getNetworkInterfaces();
>> > + while (networkInterfaces.hasMoreElements() && mac
>> == null) {
>> > + final NetworkInterface nic =
>> networkInterfaces.nextElement();
>> > + if (isUpAndNotLoopback(nic)) {
>> > + mac = nic.getHardwareAddress();
>> > + }
>> > + }
>> > + }
>> > + } catch (final SocketException e) {
>> > + LOGGER.catching(e);
>> > + }
>> > + if (mac == null || mac.length == 0) {
>> > + mac = localHost.getAddress();
>> > + }
>> > + } catch (final UnknownHostException ignored) {
>> > + }
>> > + return mac;
>> > + }
>> > +
>> > + private static boolean isUpAndNotLoopback(final NetworkInterface
>> ni) throws SocketException {
>> > + return ni != null && !ni.isLoopback() && ni.isUp();
>> > + }
>> > }
>> >
>> >
>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/2a754e12/log4j-core/src/main/java/org/apache/logging/log4j/core/util/UuidUtil.java
>> > ----------------------------------------------------------------------
>> > diff --git
>> a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/UuidUtil.java
>> b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/UuidUtil.java
>> > index 34b021f..9bed752 100644
>> > ---
>> a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/UuidUtil.java
>> > +++
>> b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/UuidUtil.java
>> > @@ -16,13 +16,8 @@
>> > */
>> > package org.apache.logging.log4j.core.util;
>> >
>> > -import java.lang.reflect.Method;
>> > -import java.net.InetAddress;
>> > -import java.net.NetworkInterface;
>> > -import java.net.UnknownHostException;
>> > import java.nio.ByteBuffer;
>> > import java.security.SecureRandom;
>> > -import java.util.Enumeration;
>> > import java.util.Random;
>> > import java.util.UUID;
>> > import java.util.concurrent.atomic.AtomicInteger;
>> > @@ -65,40 +60,7 @@ public final class UuidUtil {
>> > private static final int HUNDRED_NANOS_PER_MILLI = 10000;
>> >
>> > static {
>> > - byte[] mac = null;
>> > - try {
>> > - final InetAddress address = InetAddress.getLocalHost();
>> > - try {
>> > - NetworkInterface ni =
>> NetworkInterface.getByInetAddress(address);
>> > - if (ni != null && !ni.isLoopback() && ni.isUp()) {
>> > - final Method method =
>> ni.getClass().getMethod("getHardwareAddress");
>> > - if (method != null) {
>> > - mac = (byte[]) method.invoke(ni);
>> > - }
>> > - }
>> > -
>> > - if (mac == null) {
>> > - final Enumeration<NetworkInterface> enumeration =
>> NetworkInterface.getNetworkInterfaces();
>> > - while (enumeration.hasMoreElements() && mac ==
>> null) {
>> > - ni = enumeration.nextElement();
>> > - if (ni != null && ni.isUp() &&
>> !ni.isLoopback()) {
>> > - final Method method =
>> ni.getClass().getMethod("getHardwareAddress");
>> > - if (method != null) {
>> > - mac = (byte[]) method.invoke(ni);
>> > - }
>> > - }
>> > - }
>> > - }
>> > - } catch (final Exception ex) {
>> > - ex.printStackTrace();
>> > - // Ignore exception
>> > - }
>> > - if (mac == null || mac.length == 0) {
>> > - mac = address.getAddress();
>> > - }
>> > - } catch (final UnknownHostException e) {
>> > - // Ignore exception
>> > - }
>> > + byte[] mac = NetUtils.getLocalMacAddress();
>> > final Random randomGenerator = new SecureRandom();
>> > if (mac == null || mac.length == 0) {
>> > mac = new byte[6];
>> >
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
>> For additional commands, e-mail: log4j-dev-help@logging.apache.org
>>
>>
>
>
> --
> Matt Sicker <bo...@gmail.com>
>
>
>
--
Matt Sicker <bo...@gmail.com>
Re: git commit: Refactor MAC address getter to own method.
Posted by Ralph Goers <ra...@dslextreme.com>.
What was the reason for pulling it out of UUIDUtil in the first place? Is it going to be used somewhere else?
Ralph
On Sep 18, 2014, at 7:08 PM, Remko Popma <re...@gmail.com> wrote:
> If you anticipate using it in more places in this package, otherwise my preference would be to make it private, and increase the visibility when a new use case comes up.
>
> Sent from my iPhone
>
> On 2014/09/19, at 10:51, Matt Sicker <bo...@gmail.com> wrote:
>
>> Package-private perhaps?
>>
>> On 18 September 2014 20:01, Remko Popma <re...@gmail.com> wrote:
>> On an enterprise machine it is not uncommon to have multiple network interface cards. When used internally in UUID, it is okay to ignore that scenario, and it's also more convenient to return localhost rather than null if there is no network card at all.
>>
>> If we make this logic public, we cannot make the same assumptions any more. Shouldn't we return some sort of iterator or collection of MacAddresses? Or call it getFirstMacAddress? The Javadoc says null is returned if there's no NIC, but I don't see that in the code. (Do you anticipate using this method for something other than UUID? It might be better to leave it private...)
>>
>> Just my 2c.
>> Remko
>>
>> Sent from my iPhone
>>
>> > On 2014/09/19, at 9:05, mattsicker@apache.org wrote:
>> >
>> > Repository: logging-log4j2
>> > Updated Branches:
>> > refs/heads/master 6b80804dc -> 2a754e12b
>> >
>> >
>> > Refactor MAC address getter to own method.
>> >
>> >
>> > Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo
>> > Commit: http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/2a754e12
>> > Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/2a754e12
>> > Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/2a754e12
>> >
>> > Branch: refs/heads/master
>> > Commit: 2a754e12b28df03dab38d5e66db251094a9834a9
>> > Parents: 6b80804
>> > Author: Matt Sicker <ma...@apache.org>
>> > Authored: Thu Sep 18 19:04:56 2014 -0500
>> > Committer: Matt Sicker <ma...@apache.org>
>> > Committed: Thu Sep 18 19:04:56 2014 -0500
>> >
>> > ----------------------------------------------------------------------
>> > .../logging/log4j/core/util/NetUtils.java | 40 ++++++++++++++++++++
>> > .../logging/log4j/core/util/UuidUtil.java | 40 +-------------------
>> > 2 files changed, 41 insertions(+), 39 deletions(-)
>> > ----------------------------------------------------------------------
>> >
>> >
>> > http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/2a754e12/log4j-core/src/main/java/org/apache/logging/log4j/core/util/NetUtils.java
>> > ----------------------------------------------------------------------
>> > diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/NetUtils.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/NetUtils.java
>> > index 2609fef..46e16ce 100644
>> > --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/NetUtils.java
>> > +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/NetUtils.java
>> > @@ -70,4 +70,44 @@ public final class NetUtils {
>> > return "UNKNOWN_LOCALHOST";
>> > }
>> > }
>> > +
>> > + /**
>> > + * Returns the local network interface's MAC address if possible. The local network interface is defined here as
>> > + * the {@link NetworkInterface} that is both up and not a loopback interface.
>> > + *
>> > + * @return the MAC address of the local network interface or {@code null} if no MAC address could be determined.
>> > + * @since 2.1
>> > + */
>> > + public static byte[] getLocalMacAddress() {
>> > + byte[] mac = null;
>> > + try {
>> > + final InetAddress localHost = InetAddress.getLocalHost();
>> > + try {
>> > + final NetworkInterface localInterface = NetworkInterface.getByInetAddress(localHost);
>> > + if (isUpAndNotLoopback(localInterface)) {
>> > + mac = localInterface.getHardwareAddress();
>> > + }
>> > + if (mac == null) {
>> > + final Enumeration<NetworkInterface> networkInterfaces = NetworkInterface.getNetworkInterfaces();
>> > + while (networkInterfaces.hasMoreElements() && mac == null) {
>> > + final NetworkInterface nic = networkInterfaces.nextElement();
>> > + if (isUpAndNotLoopback(nic)) {
>> > + mac = nic.getHardwareAddress();
>> > + }
>> > + }
>> > + }
>> > + } catch (final SocketException e) {
>> > + LOGGER.catching(e);
>> > + }
>> > + if (mac == null || mac.length == 0) {
>> > + mac = localHost.getAddress();
>> > + }
>> > + } catch (final UnknownHostException ignored) {
>> > + }
>> > + return mac;
>> > + }
>> > +
>> > + private static boolean isUpAndNotLoopback(final NetworkInterface ni) throws SocketException {
>> > + return ni != null && !ni.isLoopback() && ni.isUp();
>> > + }
>> > }
>> >
>> > http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/2a754e12/log4j-core/src/main/java/org/apache/logging/log4j/core/util/UuidUtil.java
>> > ----------------------------------------------------------------------
>> > diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/UuidUtil.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/UuidUtil.java
>> > index 34b021f..9bed752 100644
>> > --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/UuidUtil.java
>> > +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/UuidUtil.java
>> > @@ -16,13 +16,8 @@
>> > */
>> > package org.apache.logging.log4j.core.util;
>> >
>> > -import java.lang.reflect.Method;
>> > -import java.net.InetAddress;
>> > -import java.net.NetworkInterface;
>> > -import java.net.UnknownHostException;
>> > import java.nio.ByteBuffer;
>> > import java.security.SecureRandom;
>> > -import java.util.Enumeration;
>> > import java.util.Random;
>> > import java.util.UUID;
>> > import java.util.concurrent.atomic.AtomicInteger;
>> > @@ -65,40 +60,7 @@ public final class UuidUtil {
>> > private static final int HUNDRED_NANOS_PER_MILLI = 10000;
>> >
>> > static {
>> > - byte[] mac = null;
>> > - try {
>> > - final InetAddress address = InetAddress.getLocalHost();
>> > - try {
>> > - NetworkInterface ni = NetworkInterface.getByInetAddress(address);
>> > - if (ni != null && !ni.isLoopback() && ni.isUp()) {
>> > - final Method method = ni.getClass().getMethod("getHardwareAddress");
>> > - if (method != null) {
>> > - mac = (byte[]) method.invoke(ni);
>> > - }
>> > - }
>> > -
>> > - if (mac == null) {
>> > - final Enumeration<NetworkInterface> enumeration = NetworkInterface.getNetworkInterfaces();
>> > - while (enumeration.hasMoreElements() && mac == null) {
>> > - ni = enumeration.nextElement();
>> > - if (ni != null && ni.isUp() && !ni.isLoopback()) {
>> > - final Method method = ni.getClass().getMethod("getHardwareAddress");
>> > - if (method != null) {
>> > - mac = (byte[]) method.invoke(ni);
>> > - }
>> > - }
>> > - }
>> > - }
>> > - } catch (final Exception ex) {
>> > - ex.printStackTrace();
>> > - // Ignore exception
>> > - }
>> > - if (mac == null || mac.length == 0) {
>> > - mac = address.getAddress();
>> > - }
>> > - } catch (final UnknownHostException e) {
>> > - // Ignore exception
>> > - }
>> > + byte[] mac = NetUtils.getLocalMacAddress();
>> > final Random randomGenerator = new SecureRandom();
>> > if (mac == null || mac.length == 0) {
>> > mac = new byte[6];
>> >
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
>> For additional commands, e-mail: log4j-dev-help@logging.apache.org
>>
>>
>>
>>
>> --
>> Matt Sicker <bo...@gmail.com>
Re: git commit: Refactor MAC address getter to own method.
Posted by Remko Popma <re...@gmail.com>.
If you anticipate using it in more places in this package, otherwise my preference would be to make it private, and increase the visibility when a new use case comes up.
Sent from my iPhone
> On 2014/09/19, at 10:51, Matt Sicker <bo...@gmail.com> wrote:
>
> Package-private perhaps?
>
>> On 18 September 2014 20:01, Remko Popma <re...@gmail.com> wrote:
>> On an enterprise machine it is not uncommon to have multiple network interface cards. When used internally in UUID, it is okay to ignore that scenario, and it's also more convenient to return localhost rather than null if there is no network card at all.
>>
>> If we make this logic public, we cannot make the same assumptions any more. Shouldn't we return some sort of iterator or collection of MacAddresses? Or call it getFirstMacAddress? The Javadoc says null is returned if there's no NIC, but I don't see that in the code. (Do you anticipate using this method for something other than UUID? It might be better to leave it private...)
>>
>> Just my 2c.
>> Remko
>>
>> Sent from my iPhone
>>
>> > On 2014/09/19, at 9:05, mattsicker@apache.org wrote:
>> >
>> > Repository: logging-log4j2
>> > Updated Branches:
>> > refs/heads/master 6b80804dc -> 2a754e12b
>> >
>> >
>> > Refactor MAC address getter to own method.
>> >
>> >
>> > Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo
>> > Commit: http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/2a754e12
>> > Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/2a754e12
>> > Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/2a754e12
>> >
>> > Branch: refs/heads/master
>> > Commit: 2a754e12b28df03dab38d5e66db251094a9834a9
>> > Parents: 6b80804
>> > Author: Matt Sicker <ma...@apache.org>
>> > Authored: Thu Sep 18 19:04:56 2014 -0500
>> > Committer: Matt Sicker <ma...@apache.org>
>> > Committed: Thu Sep 18 19:04:56 2014 -0500
>> >
>> > ----------------------------------------------------------------------
>> > .../logging/log4j/core/util/NetUtils.java | 40 ++++++++++++++++++++
>> > .../logging/log4j/core/util/UuidUtil.java | 40 +-------------------
>> > 2 files changed, 41 insertions(+), 39 deletions(-)
>> > ----------------------------------------------------------------------
>> >
>> >
>> > http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/2a754e12/log4j-core/src/main/java/org/apache/logging/log4j/core/util/NetUtils.java
>> > ----------------------------------------------------------------------
>> > diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/NetUtils.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/NetUtils.java
>> > index 2609fef..46e16ce 100644
>> > --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/NetUtils.java
>> > +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/NetUtils.java
>> > @@ -70,4 +70,44 @@ public final class NetUtils {
>> > return "UNKNOWN_LOCALHOST";
>> > }
>> > }
>> > +
>> > + /**
>> > + * Returns the local network interface's MAC address if possible. The local network interface is defined here as
>> > + * the {@link NetworkInterface} that is both up and not a loopback interface.
>> > + *
>> > + * @return the MAC address of the local network interface or {@code null} if no MAC address could be determined.
>> > + * @since 2.1
>> > + */
>> > + public static byte[] getLocalMacAddress() {
>> > + byte[] mac = null;
>> > + try {
>> > + final InetAddress localHost = InetAddress.getLocalHost();
>> > + try {
>> > + final NetworkInterface localInterface = NetworkInterface.getByInetAddress(localHost);
>> > + if (isUpAndNotLoopback(localInterface)) {
>> > + mac = localInterface.getHardwareAddress();
>> > + }
>> > + if (mac == null) {
>> > + final Enumeration<NetworkInterface> networkInterfaces = NetworkInterface.getNetworkInterfaces();
>> > + while (networkInterfaces.hasMoreElements() && mac == null) {
>> > + final NetworkInterface nic = networkInterfaces.nextElement();
>> > + if (isUpAndNotLoopback(nic)) {
>> > + mac = nic.getHardwareAddress();
>> > + }
>> > + }
>> > + }
>> > + } catch (final SocketException e) {
>> > + LOGGER.catching(e);
>> > + }
>> > + if (mac == null || mac.length == 0) {
>> > + mac = localHost.getAddress();
>> > + }
>> > + } catch (final UnknownHostException ignored) {
>> > + }
>> > + return mac;
>> > + }
>> > +
>> > + private static boolean isUpAndNotLoopback(final NetworkInterface ni) throws SocketException {
>> > + return ni != null && !ni.isLoopback() && ni.isUp();
>> > + }
>> > }
>> >
>> > http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/2a754e12/log4j-core/src/main/java/org/apache/logging/log4j/core/util/UuidUtil.java
>> > ----------------------------------------------------------------------
>> > diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/UuidUtil.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/UuidUtil.java
>> > index 34b021f..9bed752 100644
>> > --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/UuidUtil.java
>> > +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/UuidUtil.java
>> > @@ -16,13 +16,8 @@
>> > */
>> > package org.apache.logging.log4j.core.util;
>> >
>> > -import java.lang.reflect.Method;
>> > -import java.net.InetAddress;
>> > -import java.net.NetworkInterface;
>> > -import java.net.UnknownHostException;
>> > import java.nio.ByteBuffer;
>> > import java.security.SecureRandom;
>> > -import java.util.Enumeration;
>> > import java.util.Random;
>> > import java.util.UUID;
>> > import java.util.concurrent.atomic.AtomicInteger;
>> > @@ -65,40 +60,7 @@ public final class UuidUtil {
>> > private static final int HUNDRED_NANOS_PER_MILLI = 10000;
>> >
>> > static {
>> > - byte[] mac = null;
>> > - try {
>> > - final InetAddress address = InetAddress.getLocalHost();
>> > - try {
>> > - NetworkInterface ni = NetworkInterface.getByInetAddress(address);
>> > - if (ni != null && !ni.isLoopback() && ni.isUp()) {
>> > - final Method method = ni.getClass().getMethod("getHardwareAddress");
>> > - if (method != null) {
>> > - mac = (byte[]) method.invoke(ni);
>> > - }
>> > - }
>> > -
>> > - if (mac == null) {
>> > - final Enumeration<NetworkInterface> enumeration = NetworkInterface.getNetworkInterfaces();
>> > - while (enumeration.hasMoreElements() && mac == null) {
>> > - ni = enumeration.nextElement();
>> > - if (ni != null && ni.isUp() && !ni.isLoopback()) {
>> > - final Method method = ni.getClass().getMethod("getHardwareAddress");
>> > - if (method != null) {
>> > - mac = (byte[]) method.invoke(ni);
>> > - }
>> > - }
>> > - }
>> > - }
>> > - } catch (final Exception ex) {
>> > - ex.printStackTrace();
>> > - // Ignore exception
>> > - }
>> > - if (mac == null || mac.length == 0) {
>> > - mac = address.getAddress();
>> > - }
>> > - } catch (final UnknownHostException e) {
>> > - // Ignore exception
>> > - }
>> > + byte[] mac = NetUtils.getLocalMacAddress();
>> > final Random randomGenerator = new SecureRandom();
>> > if (mac == null || mac.length == 0) {
>> > mac = new byte[6];
>> >
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
>> For additional commands, e-mail: log4j-dev-help@logging.apache.org
>
>
>
> --
> Matt Sicker <bo...@gmail.com>
Re: git commit: Refactor MAC address getter to own method.
Posted by Matt Sicker <bo...@gmail.com>.
Package-private perhaps?
On 18 September 2014 20:01, Remko Popma <re...@gmail.com> wrote:
> On an enterprise machine it is not uncommon to have multiple network
> interface cards. When used internally in UUID, it is okay to ignore that
> scenario, and it's also more convenient to return localhost rather than
> null if there is no network card at all.
>
> If we make this logic public, we cannot make the same assumptions any
> more. Shouldn't we return some sort of iterator or collection of
> MacAddresses? Or call it getFirstMacAddress? The Javadoc says null is
> returned if there's no NIC, but I don't see that in the code. (Do you
> anticipate using this method for something other than UUID? It might be
> better to leave it private...)
>
> Just my 2c.
> Remko
>
> Sent from my iPhone
>
> > On 2014/09/19, at 9:05, mattsicker@apache.org wrote:
> >
> > Repository: logging-log4j2
> > Updated Branches:
> > refs/heads/master 6b80804dc -> 2a754e12b
> >
> >
> > Refactor MAC address getter to own method.
> >
> >
> > Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo
> > Commit:
> http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/2a754e12
> > Tree:
> http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/2a754e12
> > Diff:
> http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/2a754e12
> >
> > Branch: refs/heads/master
> > Commit: 2a754e12b28df03dab38d5e66db251094a9834a9
> > Parents: 6b80804
> > Author: Matt Sicker <ma...@apache.org>
> > Authored: Thu Sep 18 19:04:56 2014 -0500
> > Committer: Matt Sicker <ma...@apache.org>
> > Committed: Thu Sep 18 19:04:56 2014 -0500
> >
> > ----------------------------------------------------------------------
> > .../logging/log4j/core/util/NetUtils.java | 40 ++++++++++++++++++++
> > .../logging/log4j/core/util/UuidUtil.java | 40 +-------------------
> > 2 files changed, 41 insertions(+), 39 deletions(-)
> > ----------------------------------------------------------------------
> >
> >
> >
> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/2a754e12/log4j-core/src/main/java/org/apache/logging/log4j/core/util/NetUtils.java
> > ----------------------------------------------------------------------
> > diff --git
> a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/NetUtils.java
> b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/NetUtils.java
> > index 2609fef..46e16ce 100644
> > ---
> a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/NetUtils.java
> > +++
> b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/NetUtils.java
> > @@ -70,4 +70,44 @@ public final class NetUtils {
> > return "UNKNOWN_LOCALHOST";
> > }
> > }
> > +
> > + /**
> > + * Returns the local network interface's MAC address if possible.
> The local network interface is defined here as
> > + * the {@link NetworkInterface} that is both up and not a loopback
> interface.
> > + *
> > + * @return the MAC address of the local network interface or {@code
> null} if no MAC address could be determined.
> > + * @since 2.1
> > + */
> > + public static byte[] getLocalMacAddress() {
> > + byte[] mac = null;
> > + try {
> > + final InetAddress localHost = InetAddress.getLocalHost();
> > + try {
> > + final NetworkInterface localInterface =
> NetworkInterface.getByInetAddress(localHost);
> > + if (isUpAndNotLoopback(localInterface)) {
> > + mac = localInterface.getHardwareAddress();
> > + }
> > + if (mac == null) {
> > + final Enumeration<NetworkInterface>
> networkInterfaces = NetworkInterface.getNetworkInterfaces();
> > + while (networkInterfaces.hasMoreElements() && mac
> == null) {
> > + final NetworkInterface nic =
> networkInterfaces.nextElement();
> > + if (isUpAndNotLoopback(nic)) {
> > + mac = nic.getHardwareAddress();
> > + }
> > + }
> > + }
> > + } catch (final SocketException e) {
> > + LOGGER.catching(e);
> > + }
> > + if (mac == null || mac.length == 0) {
> > + mac = localHost.getAddress();
> > + }
> > + } catch (final UnknownHostException ignored) {
> > + }
> > + return mac;
> > + }
> > +
> > + private static boolean isUpAndNotLoopback(final NetworkInterface
> ni) throws SocketException {
> > + return ni != null && !ni.isLoopback() && ni.isUp();
> > + }
> > }
> >
> >
> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/2a754e12/log4j-core/src/main/java/org/apache/logging/log4j/core/util/UuidUtil.java
> > ----------------------------------------------------------------------
> > diff --git
> a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/UuidUtil.java
> b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/UuidUtil.java
> > index 34b021f..9bed752 100644
> > ---
> a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/UuidUtil.java
> > +++
> b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/UuidUtil.java
> > @@ -16,13 +16,8 @@
> > */
> > package org.apache.logging.log4j.core.util;
> >
> > -import java.lang.reflect.Method;
> > -import java.net.InetAddress;
> > -import java.net.NetworkInterface;
> > -import java.net.UnknownHostException;
> > import java.nio.ByteBuffer;
> > import java.security.SecureRandom;
> > -import java.util.Enumeration;
> > import java.util.Random;
> > import java.util.UUID;
> > import java.util.concurrent.atomic.AtomicInteger;
> > @@ -65,40 +60,7 @@ public final class UuidUtil {
> > private static final int HUNDRED_NANOS_PER_MILLI = 10000;
> >
> > static {
> > - byte[] mac = null;
> > - try {
> > - final InetAddress address = InetAddress.getLocalHost();
> > - try {
> > - NetworkInterface ni =
> NetworkInterface.getByInetAddress(address);
> > - if (ni != null && !ni.isLoopback() && ni.isUp()) {
> > - final Method method =
> ni.getClass().getMethod("getHardwareAddress");
> > - if (method != null) {
> > - mac = (byte[]) method.invoke(ni);
> > - }
> > - }
> > -
> > - if (mac == null) {
> > - final Enumeration<NetworkInterface> enumeration =
> NetworkInterface.getNetworkInterfaces();
> > - while (enumeration.hasMoreElements() && mac ==
> null) {
> > - ni = enumeration.nextElement();
> > - if (ni != null && ni.isUp() &&
> !ni.isLoopback()) {
> > - final Method method =
> ni.getClass().getMethod("getHardwareAddress");
> > - if (method != null) {
> > - mac = (byte[]) method.invoke(ni);
> > - }
> > - }
> > - }
> > - }
> > - } catch (final Exception ex) {
> > - ex.printStackTrace();
> > - // Ignore exception
> > - }
> > - if (mac == null || mac.length == 0) {
> > - mac = address.getAddress();
> > - }
> > - } catch (final UnknownHostException e) {
> > - // Ignore exception
> > - }
> > + byte[] mac = NetUtils.getLocalMacAddress();
> > final Random randomGenerator = new SecureRandom();
> > if (mac == null || mac.length == 0) {
> > mac = new byte[6];
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
> For additional commands, e-mail: log4j-dev-help@logging.apache.org
>
>
--
Matt Sicker <bo...@gmail.com>