> > On 3/14/2017 9:51 PM, Sergey Bylokhov wrote: >> >>> 14 марта 2017 г., в 14:41, Philip Race <[email protected] >>> <mailto:[email protected]>> написал(а): >>> >>> >Since this is mac specific code, I guess VS2010 will not play any part in >>> >building this. >>> >>> Ah, yes :-) >>> >>> Updated fix looks OK. >> >> Should we memset the data allocated via malloc(calloc was used before)? >> > Should be a good practice to do it, I guess . > Modified webrev adding memset: > http://cr.openjdk.java.net/~psadhukhan/8176287/webrev.02/ > <http://cr.openjdk.java.net/~psadhukhan/8176287/webrev.02/>
This looks fine to me, but I wonder why the calloc was replaced by malloc? > > Regards > Prasanta >>> >>> -phil. >>> >>> On 3/14/17, 2:31 AM, Prasanta Sadhukhan wrote: >>>> >>>> JPRT 8u build resulted in failure, so I had to modify at 2 other places. >>>> >>>> QuartzSurfaceData.m:287 and QuartzSurfaceData.m:328 >>>> Other things remains same. >>>> http://cr.openjdk.java.net/~psadhukhan/8176287/webrev.01/ >>>> <http://cr.openjdk.java.net/%7Epsadhukhan/8176287/webrev.01/> >>>> >>>> Regards >>>> Prasanta >>>> On 3/14/2017 10:47 AM, Philip Race wrote: >>>>> >>>>> >>>>> On 3/13/17, 10:14 PM, Prasanta Sadhukhan wrote: >>>>>> >>>>>> >>>>>> >>>>>> On 3/14/2017 10:24 AM, Philip Race wrote: >>>>>>> The problem seems to have been that you were allocating zero bytes in >>>>>>> the old code : >>>>>>> 950 CGFloat* colors= (CGFloat*)calloc(0, >>>>>>> sizeof(CGFloat)*length); >>>>>>> >>>>>>> 960 qsdo->gradientInfo->colordata = (CGFloat*)calloc(0, >>>>>>> sizeof(CGFloat)*4*length); >>>>>>> >>>>>>> Regarding the new code, whilst it seems like it fixes the problem I >>>>>>> have a nit >>>>>>> 937 int i; >>>>>>> 938 for (i=0; i<length; i++) >>>>>>> >>>>>>> Since this code appears at the start of a block I'd expect all >>>>>>> compilers to be happy with >>>>>>> for (int i=0; i<length; i++) >>>>>>> >>>>>>> is this not so ? Assuming yes, pls fix before push. >>>>>> Yes, it should be ok. I got a problem with jdk8u JPRT build (during >>>>>> earlier backport) citing C99 compiler failure but I guess that was >>>>>> because variable was declared not at blockstart. >>>>>> Will again do a JPRT and if its ok, I will push with this change. >>>>> >>>>> Testing the 8u backport via JPRT is good since that will use VS2010 which >>>>> wins the "most likely to barf" award on such an issue. >>>>> >>>>> -phil >>>>>>> >>>>>>> Also I wonder if the regression test we created for LGP passes only >>>>>>> because it is "short". >>>>>>> Perhaps later we can improve on that. >>>>>>> >>>>>>> The fix will also need to be backported since the original fix was >>>>>>> backported. >>>>>>> >>>>>> ok. >>>>>>> So "+1" with those comments .. >>>>>>> >>>>>> Thanks >>>>>> >>>>>> Regards >>>>>> Prasanta >>>>>>> -phil. >>>>>>> >>>>>>> On 3/12/17, 11:49 PM, Prasanta Sadhukhan wrote: >>>>>>>> >>>>>>>> Hi All, >>>>>>>> >>>>>>>> Please review a jck print test crash fix for jdk9. The issue was seen >>>>>>>> with only Nimbus L&F which seems to use Linear gradient path >>>>>>>> and not in other L&F (such as Aqua) . >>>>>>>> >>>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8176287 >>>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8176287> >>>>>>>> webrev: http://cr.openjdk.java.net/~psadhukhan/8176287/webrev.00/ >>>>>>>> <http://cr.openjdk.java.net/%7Epsadhukhan/8176287/webrev.00/> >>>>>>>> >>>>>>>> Linear Gradient path collects the gradient colors and fractions values >>>>>>>> in native obtained from Java and allocates several arrays to store the >>>>>>>> same in setupGradient() method. >>>>>>>> It seems even after being freed, in subsequent call to the same >>>>>>>> gradient path routine, it may get the same allocated pointer the next >>>>>>>> time the array is allocated causing it to crash citing "memory being >>>>>>>> modified after freed". >>>>>>>> >>>>>>>> Optimise setupGradient() method to allocate fewer pointer. The JCK >>>>>>>> test works now. >>>>>>>> Also, the JDK-8162796 testcase LinearGradientPrintingTest and >>>>>>>> RadialGradientPrintingTest works with this optimisation. >>>>>>>> >>>>>>>> Regards >>>>>>>> Prasanta >>>>>> >>>> >> >
