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>