Re: [OpenJDK 2D-Dev] RFR(M): 8170525: Fix minor issues in awt coding

2016-12-02 Thread Phil Race 

I had no other comments, except that it would be good to be sure
that this builds on all relevant platforms .. using the 'blessed' compilers.
If you like I can submit a JPRT job for you based on this patch.

-phil.

On 12/01/2016 11:58 PM, Lindenmaier, Goetz wrote:

Hi Phil,

I added the initialization to the other function.
http://cr.openjdk.java.net/~goetz/wr16/8170525-awt-dev/webrev.04/

I missed that because Coverity didn't complain.
It's not a perfect tool, but it finds sufficient issues
to make it worthwhile.   Is the other awt code fine?

Best regards,
   Goetz.




-Original Message-
From: Phil Race [mailto:philip.r...@oracle.com]
Sent: Donnerstag, 1. Dezember 2016 22:31
To: Lindenmaier, Goetz ; Sergey Bylokhov
; Vincent Ryan 
Cc: awt-dev@openjdk.java.net; 2d-dev <2d-...@openjdk.java.net>; security-
d...@openjdk.java.net
Subject: Re: [OpenJDK 2D-Dev]  RFR(M): 8170525: Fix minor issues
in awt coding

Sorry. it is
  ops->GetRasInfo(env, ops, lockInfo);
that initialises it ..


That is still before the dereference
Anyway, what was the reason for updating one function, but not the other.
I don't mind the change, but the inconsistency looks odd.

-phil.

On 12/01/2016 03:03 AM, Lindenmaier, Goetz wrote:

Hi Phil,


Did you actually compile this ? The variable is called "rasBase", not
"resBase".

Yes, I compiled it, and I fixed the error, but that was another repo
I use for the coverity checks.  Somehow it did not find its way into
the webrev.

I don't see where ops->Lock() initializes this field.
ops->GetRasInfo(env, ops, lockInfo); does so if it resolves
to BufImg_GetRasInfo().  I can't look in our code scan results
because the issue is gone after fixing it, that lists the path
of execution that leads to the issue.
If you are sure this is correct I will remove the initialization,
else I will also put it into the other method.

DBN_GetPixelPointer checks lockInfo->rasBase for NULL and
does what looks like the error case if it's NULL. Therefore NULL
seemed a good initialization to me.

Best regards,
Goetz.




-Original Message-
From: Phil Race [mailto:philip.r...@oracle.com]
Sent: Mittwoch, 30. November 2016 20:59
To: Sergey Bylokhov ; Lindenmaier, Goetz
; Vincent Ryan 
Cc: awt-dev@openjdk.java.net; 2d-dev <2d-...@openjdk.java.net>; security-
d...@openjdk.java.net
Subject: Re: [OpenJDK 2D-Dev]  RFR(M): 8170525: Fix minor

issues

in awt coding

Hi Goetz,


DataBufferNative.c
Using uninitialized value lockInfo.rasBase when calling

DBN_GetPixelPointer.

 75 lockInfo.resBase = NULL;

Did you actually compile this ? The variable is called "rasBase", not
"resBase".

And strictly there is no problem since inside  DBN_GetPixelPointer
the code calls ops->Lock which should initialise this.
A "rasBase" of 0 isn't really any better than a random one ..

Also I don't see why there's a problem here and not in
the function immediately following since it is the exact same case.

-phil.

On 11/30/2016 10:00 AM, Sergey Bylokhov wrote:

cc 2d-dev.

On 30.11.16 18:41, Lindenmaier, Goetz wrote:

Hi Vincent,

thanks for the quit review!
Good catch that I lost the change to p11_mutex.c ... I had to change
it and it fell out of my patches.
I edited the Last Modified Date, and also updated the copyright
messages.
New webrev:
http://cr.openjdk.java.net/~goetz/wr16/8170525-awt-dev/

Best regards,
Goetz.

(Am I correct that your openJdk name is Vinnie?)


-Original Message-
From: Vincent Ryan [mailto:vincent.x.r...@oracle.com]
Sent: Mittwoch, 30. November 2016 14:53
To: Lindenmaier, Goetz 
Cc: awt-dev@openjdk.java.net; security-...@openjdk.java.net
Subject: Re: RFR(M): 8170525: Fix minor issues in awt coding

Hello Goetz,

Please modify the bug summary to reference ECC too.
Your ECC changes look fine but the ‘Last Modified Date’ line in the
4 source
code headers will need to be updated/added.

BTW p11_mutex.c is listed below but appears to be missing from the
webrev.

Thanks.




  On 30 Nov 2016, at 13:12, Lindenmaier, Goetz
 >

wrote:

  Hi,

  I’d like to propose a row of smaller fixes where code is noted
down a
bit questionable.
  SAP’s quality process requires that we fix these in our internal
delivery,
and I
  Would like to share my fixes with openJdk.  Some of these fixes
are of
more
  theoretical nature as how I understand the code paths never
allow the
  problematic situation, but fixing it nevertheless assures that
nothing is
  overseen if the code changes.  Most changes are in libawt_xawt,
some
  are in libsunec.

  I’d appreciate a review:
  http://cr.openjdk.java.net/~goetz/wr16/8170525-awt/webrev.01/

  Changes in detail:

  awt_InputMethod.c:

  One might overrun

Re: [9] Review request for 8165428: Security Warning dialog should be always on the top when multiple applets with APPLICATION_MODAL dialog launched in a browser

2016-12-02 Thread dmitry markov 

Hi Sergey,

According to the current implementation we disable a window only when we 
are going to show a modal dialog. However I agree it is not a good idea 
to use isEnabled flag for testing whether the window is blocked or not, 
since such logic is not clear and might be accidentally broken. So I 
have updated the fix; new webrev is located at 
http://cr.openjdk.java.net/~dmarkov/8165428/webrev.01/

Summary of changes:
- Added a new function isBlocked() to CPlatformWindow class
- In AWTWindow.m use isBlocked() instead of isEnabled in the cases where 
we have to decide whether the ordering operation is required or not.


Thanks,
Dmitry
On 01/12/2016 03:29, Sergey Bylokhov wrote:

Hi, Dmitry.
Is it true that the window is disable only if blocked by some other 
window? Is it possible a situation when it can be disabled by 
application and in the same moment can have an enabled child which 
should be moved upfront?