RFR[7021373]: DatagramPacket exception conditions are not clear

[Chris Hegarty]

Patrick,

On 24/01/2020 11:14, Patrick Concannon wrote:
> Hi,
> 
> Could someone please review my fix and CSR for issue JDK-7021373 
> 'DatagramPacket exception conditions are not clear' ?
> 
> This fix updates the spec concerning the exceptions thrown by the 
> constructors of the DatagramPacket class, and several methods therein.
> 
> 
> bug: https://bugs.openjdk.java.net/browse/JDK-7021373
> 
> CSR: https://bugs.openjdk.java.net/browse/JDK-8237774
> webrev: http://cr.openjdk.java.net/~pconcannon/7021373/webrevs/webrev.00/

This is mostly good. Some pedantic comments:

1) Let's add coverage for NPE for ALL constructors, e.g.

    @Test
    public void testNull() {
        expectThrows(NPE, () -> new DatagramPacket(null, 100));
        expectThrows(NPE, () -> new DatagramPacket(null, 0, 10));
        expectThrows(NPE, () -> new DatagramPacket(null, 0, 10, 
LOOPBACK, 80));
        expectThrows(NPE, () -> new DatagramPacket(null, 10, LOOPBACK, 80));
        expectThrows(NPE, () -> new DatagramPacket(null, 0, 10, new 
InetSocketAddress(80)));
        expectThrows(NPE, () -> new DatagramPacket(null, 10, new 
InetSocketAddress(80)));
    }

2) I personally dislike the use of "data buffer" here. The invariants
    are checked on the "given" buffer. The given buffer only has an
    affect on the packet after the invariants have been checked.

3) Setters - this is a nice test. Some suggestions to improve
    readability:

    a) line up multi-line parameters vertically, e.g.

    57     @Test(dataProvider = "data")
    58     public void testSetData(byte[] buf,
                                   int offset,
                                   int length,
    59                             Class<? extends Exception> exception) {

    b) line up the provider's data to improve readability, e.g.

       95         return new Object[][]{
       96                 { -1,                IAE  },
       97                 { -666,              IAE  },
       98                 { Integer.MAX_VALUE, IAE  },
       99                 { 0xFFFF + 1,        IAE  },
      100                 { 0xFFFFFF,          IAE  },
      101                 { 0,                 null },
      102                 { 1,                 null },
      103                 { 666,               null },
      104                 { 0xFFFE,            null },
      105                 { 0xFFFF,            null },
      106         };

4) Add a NPE test for setData(byte[] buf) - I think only the 3-arg
    overload is tested?

-Chris.