Hi Andrew,
Looks good. Approved.
Jennifer
Andrew Brygin wrote:
Hello Phil,
thanks for the review.
Regarding posix_memalign(), originally I was going to use it instead of
memalign() but found that it is not available on Solaris platform.
I did not like an idea to introduce yet another platfrom-specific code
piece, and and decided to leave memaling() routine.
I do not think that there are another issues caused by the fix for
CR 7113017, and the fix suggested here probably can be used
for 7u8 (where we have to resolve the JCK failure as well).
Thanks,
Andrew
On 02.08.2012 20:40, Phil Race wrote:
I am OK with this. The submitter of the fix for 7113017 apparently
did not pay attention in the updating to use POSIX functions that
posix supplies its own function called posix_memalign so does not
define memalign. I don't know off hand what library that would come
from on all platforms and I do not think it worth cycles to figure
this out.
I hope this is the only such issue but don't know if for a fact.
Some of these portability requests seem like they are more trouble
than they are
worth and using malloc.h works just fine for me !
If there are any more issues I suggest we just revert 7113017.
-phil.
On 8/2/2012 7:36 AM, Andrew Brygin wrote:
Hello,
could you please review a fix for 7150594?
This problem is triggered by the fix for CR 7113017. In particular,
this fix replaces the malloc.h with stddef.h in mlib_types.h. This
change leads to compiling mlib_sys.c without forward declaration for
memalign() routine, and cause following warnings:
mlib_sys.c:96: warning: implicit declaration of function 'memalign'
mlib_sys.c:96: warning: cast to pointer from integer of different size
This cause the problem on systems where size of integer is less than
size of pointers: the pointer value is clamped, and usage of the
clamped pointer causes the observed crash.
Proposed solution is to include malloc.h header directly to
mlib_sys.c.
Please take a look.
Bug: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7150594
Webrev: http://cr.openjdk.java.net/~bae/7150594/8/webrev.00/
Thanks,
Andrew
