Hi Matthias,
On Fri, Jun 22, 2018 at 3:08 PM, Baesken, Matthias
<matthias.baes...@sap.com> wrote:
> Hello Alan, Thomas , I adjusted the line lengths and created a new webrev
> :
>
>
>
> http://cr.openjdk.java.net/~mbaesken/webrevs/8205342.1/
>
>
>
> I considered replacing the 100 for error_msg_buf size by a define (or
> maybe const int?) , should I do so ?
>
You may be able to do this:
#define ERRMSG "IP Helper Library GetAdaptersAddresses"
#define ERRMSG_LEN sizeof(ERRMSG)
#define ADD_LEN 30
char error_msg_buf[ERRMSG_LEN + ADD_LEN];
since sizeof(string literal) should be resolved by the preprocessor.
If that does not work, a macro as you suggested:
#define ERRMSG "IP Helper Library GetAdaptersAddresses"
#define ERRMSG_LEN 38
#define ADD_LEN 30
char error_msg_buf[ERRMSG_LEN + ADD_LEN];
Remarks:
- The memset is not needed
- As Ivan mentioned, please use sizeof(buf) instead of 100 in the call
to sprintf. IMHO however sizeof(buf)/sizeof(buf[0]) is not necessary:
we use this pattern (sizeof(buf) assuming buf = char = size 1 for
buffer output) all over the hotspot and jdk already, so if this is a
problem, we should change all those places too.
- Since your buffer will be always large enough to hold the result,
you can remove truncation handling altogether.
- I would change _snprint_s to plain snprintf because who on earth
knows what _snprintf_s() is :-)
- Matter of fact, since we cannot get truncation you may even switch
to sprintf - but that would probably make coverity complain out again.
That reduces the code to:
#define ERRMSG "IP Helper Library GetAdaptersAddresses"
#define ERRMSG_LEN 38
#define ADD_LEN 30
char error_msg_buf[ERRMSG_LEN + ADD_LEN];
snprintf_s(error_msg_buf, sizeof(error_msg_buf), "IP Helper Library
GetAdaptersAddresses "
"function failed with error == %d", ret);
JNU_ThrowByName(env, "java/lang/Error", error_msg_buf);
---
Beside all that I still think that the least invasive and most clear
patch by far for your problem - the memory leak - would have been to
just add the two missing free() calls...
Best Regards, Thomas
>
>
>
>
> Best regards, Matthias
>
>
>
>
>
> From: Alan Bateman [mailto:alan.bate...@oracle.com]
> Sent: Mittwoch, 20. Juni 2018 10:45
> To: Baesken, Matthias <matthias.baes...@sap.com>; net-dev@openjdk.java.net
> Subject: Re: RFR: 8205342: windows : potential memleaks in getAdapter(s) in
> NetworkInterface_winXP.c
>
>
>
>
>
> On 20/06/2018 09:07, Baesken, Matthias wrote:
>
> Hello . Please review this small fix that fixes potential memory leaks
> in getAdapter(s) in NetworkInterface_winXP.c and simplifies the coding a
> bit too .
>
> Currently when generating error messages , some memory is malloc-ed
> for the error messages , but not always freed .
>
>
>
>
>
> Bug:
>
>
>
> https://bugs.openjdk.java.net/browse/JDK-8205342
>
>
>
> webrev :
>
>
>
> http://cr.openjdk.java.net/~mbaesken/webrevs/8205342/
>
>
>
> Can you fix the line lengths to make it consistent with original code? That
> will make it easier to look at side-by-side diffs.
>
> -Alan
