RFR: 8170544: Fix code scan findings in libnet
Lindenmaier, Goetz
goetz.lindenmaier at sap.com
Thu Dec 29 10:07:41 UTC 2016
Hi Christoph,
I had a look at your change.
It's a bit confusing that you call the variable 'sa' everywhere, as
a field of it has the same name. sa.sa looks confusing.
But it's good that you name it everywhere the same, and
that you went through all this code.
Some details:
http://cr.openjdk.java.net/~clanger/webrevs/8170544.0/src/java.base/share/native/libnet/net_util.c.udiff.html
+ * check now to whether we have IPv6 on this platform and if the
superfluous 'to'
http://cr.openjdk.java.net/~clanger/webrevs/8170544.0/src/java.base/unix/native/libnet/NetworkInterface.c.udiff.html
For the records: you add closing the connection in an error case.
http://cr.openjdk.java.net/~clanger/webrevs/8170544.0/src/java.base/unix/native/libnet/net_util_md.c.udiff.html
Is it safe to do memset here? I think memset in the ipv4/v6 cases with the corresponding sizes is safer.
Len is not passed in all the times. Else you could memset with len.
(You change len to '0' in http://cr.openjdk.java.net/~clanger/webrevs/8170544.0/src/java.base/unix/native/libnio/ch/DatagramChannelImpl.c.udiff.html )
If you are sure you always pass a full SOCKETADDRESS this is fine, though.
Same holds for the windows size.
Overall, the len field is quite superfluous now, isn't it? But this should not be changed
in this change I think.
http://cr.openjdk.java.net/~clanger/webrevs/8170544.0/src/java.base/windows/native/libnet/TwoStacksPlainDatagramSocketImpl.c.udiff.html
Please add spaces around '=': fd=-1
Otherwise looks good.
I ran the change through our code scan, the issues are all gone now.
Best regards,
Goetz.
> -----Original Message-----
> From: Langer, Christoph
> Sent: Donnerstag, 1. Dezember 2016 12:54
> To: Michael McMahon <michael.x.mcmahon at oracle.com>
> Cc: net-dev at openjdk.java.net; Chris Hegarty <chris.hegarty at oracle.com>;
> Lindenmaier, Goetz <goetz.lindenmaier at sap.com>
> Subject: RE: RFR: 8170544: Fix code scan findings in libnet
>
> Hi Michael,
>
>
>
> I have just updated http://cr.openjdk.java.net/~clanger/webrevs/8170544.0/ in
> place. The old version caused a compile warning on Linux.
>
>
>
> Best regards
>
> Christoph
>
>
>
> From: Langer, Christoph
> Sent: Mittwoch, 30. November 2016 19:01
> To: 'Michael McMahon' <michael.x.mcmahon at oracle.com>
> Cc: net-dev at openjdk.java.net; Chris Hegarty <chris.hegarty at oracle.com>;
> Lindenmaier, Goetz <goetz.lindenmaier at sap.com>
> Subject: RE: RFR: 8170544: Fix code scan findings in libnet
>
>
>
> Thanks, Michael.
>
>
>
> Maybe you can run all the 3 together:
>
>
>
> http://cr.openjdk.java.net/~clanger/webrevs/8167457.1/
>
> http://cr.openjdk.java.net/~clanger/webrevs/8167420.2/
> <http://cr.openjdk.java.net/~clanger/webrevs/8167420.2/>
>
> http://cr.openjdk.java.net/~clanger/webrevs/8170544.0/
>
>
>
> 8170544 comes atop 8167420 (though it's just a small clash).
>
>
>
> Best regards
>
> Christoph
>
>
>
>
>
> From: Michael McMahon [mailto:michael.x.mcmahon at oracle.com]
> Sent: Mittwoch, 30. November 2016 17:57
> To: Langer, Christoph <christoph.langer at sap.com
> <mailto:christoph.langer at sap.com> >
> Cc: net-dev at openjdk.java.net <mailto:net-dev at openjdk.java.net> ; Chris
> Hegarty <chris.hegarty at oracle.com <mailto:chris.hegarty at oracle.com> >;
> Lindenmaier, Goetz <goetz.lindenmaier at sap.com
> <mailto:goetz.lindenmaier at sap.com> >
> Subject: Re: RFR: 8170544: Fix code scan findings in libnet
>
>
>
> I will run the change through JPRT Christoph
>
> Thanks
> Michael
>
> On 30/11/2016, 16:32, Langer, Christoph wrote:
>
> Hi,
>
>
>
> please review the following change:
>
>
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8170544
>
> Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8170544.0/
> <http://cr.openjdk.java.net/%7Eclanger/webrevs/8170544.0/>
>
>
>
> With this change I change some function signatures to use
> SOCKETADDRESS * instead of struct sockaddr * where appropriate. This fixes
> some code scan findings, which reported the fact that it's dangerous to pass
> struct socaddr * pointers and then cast it to the wider struct sockaddr_in6. I
> think this makes the code cleaner and spares some casts at several places.
>
>
>
> Please review and run through JPRT testing.
>
>
>
> Thanks
>
> Christoph
>
>
>
>
More information about the net-dev
mailing list