Hi Matthias,
+1
Thanks,
Serguei
On 12/12/19 2:00 AM, Langer, Christoph wrote:
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/
