Hi Matthias,
I think your current patch is good as it is – at least it wouldn’t make things
worse, AFAICS.
Further improvements can probably be done under another issue.
Cheers
Christoph
From: serviceability-dev <serviceability-dev-boun...@openjdk.java.net> On
Behalf Of Baesken, Matthias
Sent: Freitag, 29. November 2019 08:18
To: Thomas Stüfe <thomas.stu...@gmail.com>
Cc: serviceability-dev@openjdk.java.net
Subject: [CAUTION] RE: RFR [XS]: 8234968: check calloc rv in libinstrument
InvocationAdapter
Hi Thomas, Christoph, thanks for the comments . Of course the init of *
decodedLen must be added .
In case of returning NULL from decodePath , we would have tmp == NULL
(in char* tmp = func; ) , assign tmp to res and then we
jplis_assert , see :
#define TRANSFORM(res,func) { \
char* tmp = func; \
if (tmp != res) { \
free(res); \
res = tmp; \
} \
jplis_assert((void*)res != (void*)NULL); \
}
….
TRANSFORM(path, decodePath(path,&len));
New webrev :
http://cr.openjdk.java.net/~mbaesken/webrevs/8234968.2/
Best regards, Matthias
From: Thomas Stüfe <thomas.stu...@gmail.com<mailto:thomas.stu...@gmail.com>>
Sent: Freitag, 29. November 2019 07:30
To: Baesken, Matthias
<matthias.baes...@sap.com<mailto:matthias.baes...@sap.com>>
Cc:
serviceability-dev@openjdk.java.net<mailto:serviceability-dev@openjdk.java.net>
Subject: Re: RFR [XS]: 8234968: check calloc rv in libinstrument
InvocationAdapter
Hi Matthias,
I am not certain the callers are prepared to handle NULL.
This is used in a chain of TRANSFORM macro calls which AFAICS do not handle
NULL; e.g. , at 872, we pass the returned pointer to
convertUft8ToPlatformString which passes it on (on Windows) to
MultiByteToWideChar, which does not handle NULL input.
So I wonder whether a clear error message with an exit would be better in this
case. Otherwise we may get a crash just some instructions later.
Cheers, Thomas
On Thu, Nov 28, 2019 at 5:21 PM Baesken, Matthias
<matthias.baes...@sap.com<mailto:matthias.baes...@sap.com>> wrote:
Hello, please review this small patch .
It adds return value checking for calloc at one place where it is missing .
Thanks, Matthias
Bug/webrev :
https://bugs.openjdk.java.net/browse/JDK-8234968
http://cr.openjdk.java.net/~mbaesken/webrevs/8234968.1/
