Skip to content

Commit

Permalink
Enable MagicNumber checkstyle module
Browse files Browse the repository at this point in the history
Genymobile requires MagicNumber to be enabled.

However, it produces many false-positives, especially with some bitwise
operations (widely used in this project):

    x = value & 0xff;
    x = value & 0xf000;
    x = value >> 16;

Extracting 16, 0xff and 0xf000 to separate constants would be insane in
these examples.

It also consider many numbers as "magic" in tests, where we don't want
to extract them to a separate constant:

    int sum = add(12, 34);
    assertEquals(46, sum);

Therefore, handle these specific cases by disabling the check locally
through @SuppressWarnings annotations.
  • Loading branch information
rom1v committed Mar 28, 2017
1 parent c019c91 commit 26add7b
Show file tree
Hide file tree
Showing 24 changed files with 38 additions and 5 deletions.
1 change: 1 addition & 0 deletions app/src/main/java/com/genymobile/gnirehtet/Binary.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package com.genymobile.gnirehtet;

@SuppressWarnings("checkstyle:MagicNumber")
public final class Binary {

private Binary() {
Expand Down
7 changes: 5 additions & 2 deletions app/src/main/java/com/genymobile/gnirehtet/Forwarder.java
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ public class Forwarder {

private static final int BUFSIZE = 4096;

private static final byte[] DUMMY_ADDRESS = {42, 42, 42, 42};
private static final int DUMMY_PORT = 4242;

private final FileDescriptor vpnFileDescriptor;
private final Tunnel tunnel;

Expand Down Expand Up @@ -143,8 +146,8 @@ private void wakeUpReadWorkaround() {
public void run() {
try {
DatagramSocket socket = new DatagramSocket();
InetAddress addr = InetAddress.getByAddress(new byte[] {42, 42, 42, 42});
DatagramPacket packet = new DatagramPacket(new byte[0], 0, addr, 4242);
InetAddress dummyAddr = InetAddress.getByAddress(DUMMY_ADDRESS);
DatagramPacket packet = new DatagramPacket(new byte[0], 0, dummyAddr, DUMMY_PORT);
socket.send(packet);
} catch (IOException e) {
// ignore
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ private void startVpn(VpnConfiguration config) {
startForwarding();
}

@SuppressWarnings("checkstyle:MagicNumber")
private void setupVpn(VpnConfiguration config) {
Builder builder = new Builder();
builder.addAddress(VPN_ADDRESS, 32);
Expand All @@ -114,6 +115,7 @@ private void setupVpn(VpnConfiguration config) {
setAsUndernlyingNetwork();
}

@SuppressWarnings("checkstyle:MagicNumber")
private void setAsUndernlyingNetwork() {
if (Build.VERSION.SDK_INT >= 22) {
Network vpnNetwork = findVpnNetwork();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
/**
* Wrapper for writing one IP packet at a time to an {@link OutputStream}.
*/
@SuppressWarnings("checkstyle:MagicNumber")
public class IPPacketOutputStream extends OutputStream {

private static final String TAG = IPPacketOutputStream.class.getSimpleName();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
*/
public class RelayTunnelProvider {

private static final int DELAY_BETWEEN_ATTEMPTS_MS = 5000;

private final VpnService vpnService;
private RelayTunnel tunnel;
private boolean first = true;
Expand All @@ -36,8 +38,7 @@ public RelayTunnelProvider(VpnService vpnService) {
public synchronized RelayTunnel getCurrentTunnel() throws IOException, InterruptedException {
if (tunnel == null) {
if (!first) {
// add delay between attempts
Thread.sleep(5000);
Thread.sleep(DELAY_BETWEEN_ATTEMPTS_MS);
} else {
first = false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import java.nio.ByteBuffer;
import java.util.Arrays;

@SuppressWarnings("checkstyle:MagicNumber")
public class TestIPPacketOutputSteam {

private ByteBuffer createMockPacket() {
Expand Down
5 changes: 5 additions & 0 deletions config/checkstyle/checkstyle.xml
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,11 @@ page at http://checkstyle.sourceforge.net/config.html -->
</module>
<module name="IllegalInstantiation" />
<module name="InnerAssignment" />
<module name="MagicNumber">
<property name="severity" value="info" />
<property name="ignoreHashCodeMethod" value="true" />
<property name="ignoreAnnotation" value="true" />
</module>
<module name="MissingSwitchDefault" />
<module name="SimplifyBooleanExpression" />
<module name="SimplifyBooleanReturn" />
Expand Down
1 change: 1 addition & 0 deletions relay/src/main/java/com/genymobile/relay/Binary.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import java.nio.ByteBuffer;

@SuppressWarnings("checkstyle:MagicNumber")
public final class Binary {

private Binary() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
* head tail
* </pre>
*/
@SuppressWarnings("checkstyle:MagicNumber")
public class DatagramBuffer {

private static final String TAG = DatagramBuffer.class.getSimpleName();
Expand Down
5 changes: 4 additions & 1 deletion relay/src/main/java/com/genymobile/relay/IPv4Header.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import java.nio.ByteBuffer;

@SuppressWarnings("checkstyle:MagicNumber")
public class IPv4Header {

public enum Protocol {
Expand All @@ -44,6 +45,8 @@ static Protocol fromNumber(int number) {
}
}

private static final int MIN_IPV4_HEADER_LENGTH = 20;

private ByteBuffer raw;
private byte version;
private int headerLength;
Expand All @@ -53,7 +56,7 @@ static Protocol fromNumber(int number) {
private int destination;

public IPv4Header(ByteBuffer raw) {
assert raw.limit() >= 20 : "IPv4 headers length must be at least 20 bytes";
assert raw.limit() >= MIN_IPV4_HEADER_LENGTH : "IPv4 headers length must be at least 20 bytes";
this.raw = raw;

byte versionAndIHL = raw.get(0);
Expand Down
1 change: 1 addition & 0 deletions relay/src/main/java/com/genymobile/relay/IPv4Packet.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ public class IPv4Packet {

private static final String TAG = IPv4Packet.class.getSimpleName();

@SuppressWarnings("checkstyle:MagicNumber")
public static final int MAX_PACKET_LENGTH = 1 << 16; // packet length is stored on 16 bits

private final ByteBuffer raw;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ public int readFrom(ReadableByteChannel channel) throws IOException {
return channel.read(buffer);
}

@SuppressWarnings("checkstyle:MagicNumber")
private int getAvailablePacketLength() {
int length = IPv4Header.readLength(buffer);
assert length == -1 || IPv4Header.readVersion(buffer) == 4 : "This function must not be called when the packet is not IPv4";
Expand Down
1 change: 1 addition & 0 deletions relay/src/main/java/com/genymobile/relay/Net.java
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ public static InetAddress toInetAddress(byte[] raw) {
}
}

@SuppressWarnings("checkstyle:MagicNumber")
public static InetAddress toInetAddress(int ipAddr) {
byte[] ip = {
(byte) (ipAddr >>> 24),
Expand Down
1 change: 1 addition & 0 deletions relay/src/main/java/com/genymobile/relay/TCPHeader.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import java.nio.ByteBuffer;

@SuppressWarnings("checkstyle:MagicNumber")
public class TCPHeader implements TransportHeader {

public static final int FLAG_FIN = 1 << 0;
Expand Down
1 change: 1 addition & 0 deletions relay/src/main/java/com/genymobile/relay/UDPHeader.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import java.nio.ByteBuffer;

@SuppressWarnings("checkstyle:MagicNumber")
public class UDPHeader implements TransportHeader {

private static final int UDP_HEADER_LENGTH = 8;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import java.nio.channels.Channels;
import java.nio.channels.WritableByteChannel;

@SuppressWarnings("checkstyle:MagicNumber")
public class DatagramBufferTest {

private static ByteBuffer createDatagram(int size) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

import java.nio.ByteBuffer;

@SuppressWarnings("checkstyle:MagicNumber")
public class IPv4HeaderTest {

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import java.nio.channels.Channels;
import java.nio.channels.ReadableByteChannel;

@SuppressWarnings("checkstyle:MagicNumber")
public class IPv4PacketBufferTest {

private static ByteBuffer createMockPacket() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

import java.nio.ByteBuffer;

@SuppressWarnings("checkstyle:MagicNumber")
public class IPv4PacketTest {

private static ByteBuffer createMockPacket() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

import java.net.InetAddress;

@SuppressWarnings("checkstyle:MagicNumber")
public class InetAddressTest {

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import java.nio.channels.Channels;
import java.nio.channels.ReadableByteChannel;

@SuppressWarnings("checkstyle:MagicNumber")
public class PacketizerTest {

private static ByteBuffer createMockPacket() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import java.nio.channels.Channels;
import java.nio.channels.WritableByteChannel;

@SuppressWarnings("checkstyle:MagicNumber")
public class StreamBufferTest {

private static ByteBuffer createChunk() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

import java.nio.ByteBuffer;

@SuppressWarnings("checkstyle:MagicNumber")
public class TCPHeaderTest {

private static ByteBuffer createMockPacket() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

import java.nio.ByteBuffer;

@SuppressWarnings("checkstyle:MagicNumber")
public class UDPHeaderTest {

private static ByteBuffer createMockHeaders() {
Expand Down

0 comments on commit 26add7b

Please sign in to comment.