[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-2d-dev
Subject:    [OpenJDK 2D-Dev] [8] request for review: 7150594: VM chash in JCK api/java_awt/Image/ConvolveOp/ tes
From:       jennifer.godinez () oracle ! com (Jennifer Godinez)
Date:       2012-08-09 17:02:04
Message-ID: 5023ED0C.4020701 () oracle ! com
[Download RAW message or body]

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
>>
>

[prev in list] [next in list] [prev in thread] [next in thread]