I normally do not comment on component source code changes, but I glanced 
through this and noticed that a lot of size_t values are casted to int, in 
situations where a size_t is expected, like SAFE_ALLOC or so. Perhaps it would 
be better to change the argument to those functions, rather than to cast a lot 
of size_t expressions to int. As a rule of thumb, any expression that measure 
an amount of memory should be of type size_t, rather than int. 

/Magnus

> 27 nov. 2018 kl. 12:14 skrev Krishna Addepalli <krishna.addepa...@oracle.com>:
> 
> Hi Phil,
>  
> To reduce the scope, I have created a new webrev, which addresses only 
> warnings on Linux platform.
> Warnings for other platforms will be addressed in separate bugs.
> Here is the new webrev: 
> http://cr.openjdk.java.net/~kaddepalli/8074824/webrev02/
>  
> For your reference, I’m attaching the warning log generated by the compiler 
> for each warning type. Hope this helps in the review.
> I ran the all the jtreg tests, but I’m not sure if the changes have caused 
> any problems. 
> I checked with Ajit (who tried to address this issue before), and ran 
> SwingSet2 with GTK2 and GTK3 and did not find any crashes.
>  
> Thanks,
> Krishna
>  
> From: Krishna Addepalli 
> Sent: Tuesday, October 2, 2018 8:53 PM
> To: Philip Race <philip.r...@oracle.com>
> Cc: awt-...@openjdk.java.net; 2d-dev <2d-dev@openjdk.java.net>; build-dev 
> <build-...@openjdk.java.net>
> Subject: Re: [OpenJDK 2D-Dev] <AWT Dev> [12]RFR: [JDK-8074824]: Resolve 
> disabled warnings for libawt_xawt
>  
> Yes, that is right.
> I have compiled it Mac, Linux and Windows locally. I tried submitting a Mach5 
> job, but was unable to as it was down. Will try it again.
>  
> Thanks
> Krishna
> 
> 
> On 02-Oct-2018, at 3:39 AM, Philip Race <philip.r...@oracle.com> wrote:
>  
> I suspect I understand this one now .. the array is stack allocated so we 
> don't want NULL
> but the compiler probably complained about possible uninitialised use of the 
> values ?
> 
> -phil.
> 
> On 10/1/18, 9:38 AM, Philip Race wrote:
> You really do need to explain *each* of the changes better.
> This one .. why not NULL ?
> http://cr.openjdk.java.net/~kaddepalli/8074824/webrev01/src/java.desktop/share/native/libawt/java2d/loops/ProcessPath.c.udiff.html
> 
> -phil
> 
> On 10/1/18, 9:19 AM, Philip Race wrote:
> Hi,
> 
> 1) Has this been built on all platforms ?
> 2)  I can't find the list of warnings that you are seeing and fixing and they 
> are all over the place.
> So adding 2d-dev and build-dev.
> For each of these changes, please show what was the warning that you received 
> from the compiler
> This might sound like a lot of work, but it won't be disproportionate and 
> I've made the same
> request for similar reviews and without it, it is hard to review the changes.
> 
> For example (and I do mean just example) 
> http://cr.openjdk.java.net/~kaddepalli/8074824/webrev01/src/java.desktop/unix/native/common/awt/awt_Font.c.udiff.html
> 
> why would that not be #ifdef instead ?
> 
> 3) Testing .. did you run at least all our jtreg tests to make sure you 
> didn't break
> some behaviour ..
> 
> -phil.
> 
> On 9/29/18, 8:18 PM, Krishna Addepalli wrote:
> Hi All, 
>  
> Please review a fix for JDK-8074824: 
> https://bugs.openjdk.java.net/browse/JDK-8074824
> Webrev: http://cr.openjdk.java.net/~kaddepalli/8074824/webrev01/
>  
> Most of the warnings have been fixed for Linux, Mac and Windows.
>  
> Thanks,
> Krishna
>  
> <WarningInfo.txt>

Reply via email to