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

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

2016-12-06 Thread Lindenmaier, Goetz

Hi Phil, 

does that mean I can push the change, or will this happen through jprt?

Best regards,
  Goetz.

> -Original Message-
> From: Philip Race [mailto:philip.r...@oracle.com]
> Sent: Tuesday, December 06, 2016 4:21 AM
> To: Lindenmaier, Goetz <goetz.lindenma...@sap.com>
> Cc: Sergey Bylokhov <sergey.bylok...@oracle.com>; awt-
> d...@openjdk.java.net; 2d-dev <2d-dev@openjdk.java.net>
> Subject: Re: [OpenJDK 2D-Dev]  RFR(M): 8170525: Fix minor
> issues in awt coding
> 
> I didn't eyeball what you changed but JPRT is now happy.
> The build passes on all platforms...
> 
> -phil.
> 
> On 12/5/16, 2:47 PM, Lindenmaier, Goetz wrote:
> > Hi Phil,
> >
> > sorry for that! I fixed it, there is an other occurrence, too.
> > I double checked all the casts, there was also a mp_flags<->  mp_sign
> wrong in mpi.c
> > http://cr.openjdk.java.net/~goetz/wr16/8170525-awt-dev/webrev.05/
> > (Also rebased the change)
> >
> > Best regards,
> >Goetz
> >
> >> -Original Message-
> >> From: Phil Race [mailto:philip.r...@oracle.com]
> >> Sent: Monday, December 05, 2016 7:26 PM
> >> To: Lindenmaier, Goetz<goetz.lindenma...@sap.com>; Sergey Bylokhov
> >> <sergey.bylok...@oracle.com>
> >> Cc: awt-...@openjdk.java.net; 2d-dev<2d-dev@openjdk.java.net>
> >> Subject: Re: [OpenJDK 2D-Dev]  RFR(M): 8170525: Fix minor
> >> issues in awt coding
> >>
> >> I tried it .. and just as well I did. It fails in the crypto code on Mac.
> >>
> >> jdk/src/jdk.crypto.ec/share/native/libsunec/impl/ec.c:261:12: error:
> >> expression which evaluates to zero treated as a null pointer constant of
> type
> >> 'mp_digit *' (aka 'unsigned long *') 
> >> [-Werror,-Wnon-literal-null-conversion]
> >>   k.dp = (mp_digit)0;
> >>  ^~~
> >> 1 error generated.
> >>
> >> -phil.
> >>
> >>
> >>
> >> On 12/3/2016 9:53 AM, Lindenmaier, Goetz wrote:
> >>> Hi Phil,
> >>>
> >>> that would be great, unfortunately I don't have access to JPRT.
> >>>
> >>> Thanks,
> >>> Goetz.
> >>>
> >>>> -Original Message-
> >>>> From: Phil Race [mailto:philip.r...@oracle.com]
> >>>> Sent: Friday, December 02, 2016 8:46 PM
> >>>> To: Lindenmaier, Goetz<goetz.lindenma...@sap.com>; Sergey
> Bylokhov
> >>>> <sergey.bylok...@oracle.com>
> >>>> Cc: awt-...@openjdk.java.net; 2d-dev<2d-dev@openjdk.java.net>
> >>>> Subject: Re: [OpenJDK 2D-Dev]  RFR(M): 8170525: Fix minor
> >>>> issues in awt coding
> >>>>
> >>>> 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<goetz.lindenma...@sap.com>; Sergey
> >> Bylokhov
> >>>>>> <sergey.bylok...@oracle.com>; Vincent Ryan
> >>>> <vincent.x.r...@oracle.com>
> >>>>>> Cc: awt-...@openjdk.java.net; 2d-dev<2d-dev@openjdk.java.net>;
> >>>> security-
> >>>>>> d...@openjdk.java.net
> >>>>>> Subject: Re: [OpenJDK 2D-Dev]  RFR(M): 8170525: Fix
> minor
> >>>> issues
> >>>>>> in awt coding
> >>>>>>
> >>>>&

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

2016-12-05 Thread Philip Race


I didn't eyeball what you changed but JPRT is now happy.
The build passes on all platforms...

-phil.

On 12/5/16, 2:47 PM, Lindenmaier, Goetz wrote:

Hi Phil,

sorry for that! I fixed it, there is an other occurrence, too.
I double checked all the casts, there was also a mp_flags<->  mp_sign wrong in 
mpi.c
http://cr.openjdk.java.net/~goetz/wr16/8170525-awt-dev/webrev.05/
(Also rebased the change)

Best regards,
   Goetz


-Original Message-
From: Phil Race [mailto:philip.r...@oracle.com]
Sent: Monday, December 05, 2016 7:26 PM
To: Lindenmaier, Goetz<goetz.lindenma...@sap.com>; Sergey Bylokhov
<sergey.bylok...@oracle.com>
Cc: awt-...@openjdk.java.net; 2d-dev<2d-dev@openjdk.java.net>
Subject: Re: [OpenJDK 2D-Dev]  RFR(M): 8170525: Fix minor
issues in awt coding

I tried it .. and just as well I did. It fails in the crypto code on Mac.

jdk/src/jdk.crypto.ec/share/native/libsunec/impl/ec.c:261:12: error:
expression which evaluates to zero treated as a null pointer constant of type
'mp_digit *' (aka 'unsigned long *') [-Werror,-Wnon-literal-null-conversion]
  k.dp = (mp_digit)0;
 ^~~
1 error generated.

-phil.



On 12/3/2016 9:53 AM, Lindenmaier, Goetz wrote:

Hi Phil,

that would be great, unfortunately I don't have access to JPRT.

Thanks,
Goetz.


-Original Message-
From: Phil Race [mailto:philip.r...@oracle.com]
Sent: Friday, December 02, 2016 8:46 PM
To: Lindenmaier, Goetz<goetz.lindenma...@sap.com>; Sergey Bylokhov
<sergey.bylok...@oracle.com>
Cc: awt-...@openjdk.java.net; 2d-dev<2d-dev@openjdk.java.net>
Subject: Re: [OpenJDK 2D-Dev]  RFR(M): 8170525: Fix minor
issues in awt coding

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<goetz.lindenma...@sap.com>; Sergey

Bylokhov

<sergey.bylok...@oracle.com>; Vincent Ryan

<vincent.x.r...@oracle.com>

Cc: awt-...@openjdk.java.net; 2d-dev<2d-dev@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<sergey.bylok...@oracle.com>; Lindenmaier,

Goetz

<goetz.lindenma...@sap.com>; Vincent Ryan

<vincent.x.r...@oracle.com>

Cc: awt-...@openjdk.java.net; 2d-dev<2d-dev@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, S

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

2016-12-05 Thread Lindenmaier, Goetz

Hi Phil, 

sorry for that! I fixed it, there is an other occurrence, too.
I double checked all the casts, there was also a mp_flags <-> mp_sign wrong in 
mpi.c
http://cr.openjdk.java.net/~goetz/wr16/8170525-awt-dev/webrev.05/
(Also rebased the change)

Best regards,
  Goetz

> -Original Message-
> From: Phil Race [mailto:philip.r...@oracle.com]
> Sent: Monday, December 05, 2016 7:26 PM
> To: Lindenmaier, Goetz <goetz.lindenma...@sap.com>; Sergey Bylokhov
> <sergey.bylok...@oracle.com>
> Cc: awt-...@openjdk.java.net; 2d-dev <2d-dev@openjdk.java.net>
> Subject: Re: [OpenJDK 2D-Dev]  RFR(M): 8170525: Fix minor
> issues in awt coding
> 
> I tried it .. and just as well I did. It fails in the crypto code on Mac.
> 
> jdk/src/jdk.crypto.ec/share/native/libsunec/impl/ec.c:261:12: error:
> expression which evaluates to zero treated as a null pointer constant of type
> 'mp_digit *' (aka 'unsigned long *') [-Werror,-Wnon-literal-null-conversion]
>  k.dp = (mp_digit)0;
> ^~~
> 1 error generated.
> 
> -phil.
> 
> 
> 
> On 12/3/2016 9:53 AM, Lindenmaier, Goetz wrote:
> > Hi Phil,
> >
> > that would be great, unfortunately I don't have access to JPRT.
> >
> > Thanks,
> >Goetz.
> >
> >> -Original Message-
> >> From: Phil Race [mailto:philip.r...@oracle.com]
> >> Sent: Friday, December 02, 2016 8:46 PM
> >> To: Lindenmaier, Goetz <goetz.lindenma...@sap.com>; Sergey Bylokhov
> >> <sergey.bylok...@oracle.com>
> >> Cc: awt-...@openjdk.java.net; 2d-dev <2d-dev@openjdk.java.net>
> >> Subject: Re: [OpenJDK 2D-Dev]  RFR(M): 8170525: Fix minor
> >> issues in awt coding
> >>
> >> 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 <goetz.lindenma...@sap.com>; Sergey
> Bylokhov
> >>>> <sergey.bylok...@oracle.com>; Vincent Ryan
> >> <vincent.x.r...@oracle.com>
> >>>> Cc: awt-...@openjdk.java.net; 2d-dev <2d-dev@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.
&g

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

2016-12-05 Thread Phil Race


I tried it .. and just as well I did. It fails in the crypto code on Mac.

jdk/src/jdk.crypto.ec/share/native/libsunec/impl/ec.c:261:12: error: expression 
which evaluates to zero treated as a null pointer constant of type 'mp_digit *' 
(aka 'unsigned long *') [-Werror,-Wnon-literal-null-conversion]
k.dp = (mp_digit)0;
   ^~~
1 error generated.

-phil.



On 12/3/2016 9:53 AM, Lindenmaier, Goetz wrote:

Hi Phil,

that would be great, unfortunately I don't have access to JPRT.

Thanks,
   Goetz.
  

-Original Message-
From: Phil Race [mailto:philip.r...@oracle.com]
Sent: Friday, December 02, 2016 8:46 PM
To: Lindenmaier, Goetz <goetz.lindenma...@sap.com>; Sergey Bylokhov
<sergey.bylok...@oracle.com>
Cc: awt-...@openjdk.java.net; 2d-dev <2d-dev@openjdk.java.net>
Subject: Re: [OpenJDK 2D-Dev]  RFR(M): 8170525: Fix minor
issues in awt coding

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 <goetz.lindenma...@sap.com>; Sergey Bylokhov
<sergey.bylok...@oracle.com>; Vincent Ryan

<vincent.x.r...@oracle.com>

Cc: awt-...@openjdk.java.net; 2d-dev <2d-dev@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 <sergey.bylok...@oracle.com>; Lindenmaier,

Goetz

<goetz.lindenma...@sap.com>; Vincent Ryan

<vincent.x.r...@oracle.com>

Cc: awt-...@openjdk.java.net; 2d-dev <2d-dev@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 <goetz.lindenma...@sap.com>
Cc: awt-...@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.

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

2016-12-03 Thread Lindenmaier, Goetz

Hi Phil, 

that would be great, unfortunately I don't have access to JPRT.

Thanks,
  Goetz.
 
> -Original Message-
> From: Phil Race [mailto:philip.r...@oracle.com]
> Sent: Friday, December 02, 2016 8:46 PM
> To: Lindenmaier, Goetz <goetz.lindenma...@sap.com>; Sergey Bylokhov
> <sergey.bylok...@oracle.com>
> Cc: awt-...@openjdk.java.net; 2d-dev <2d-dev@openjdk.java.net>
> Subject: Re: [OpenJDK 2D-Dev]  RFR(M): 8170525: Fix minor
> issues in awt coding
> 
> 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 <goetz.lindenma...@sap.com>; Sergey Bylokhov
> >> <sergey.bylok...@oracle.com>; Vincent Ryan
> <vincent.x.r...@oracle.com>
> >> Cc: awt-...@openjdk.java.net; 2d-dev <2d-dev@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 <sergey.bylok...@oracle.com>; Lindenmaier,
> Goetz
> >>>> <goetz.lindenma...@sap.com>; Vincent Ryan
> <vincent.x.r...@oracle.com>
> >>>> Cc: awt-...@openjdk.java.net; 2d-dev <2d-dev@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.
&g

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 <goetz.lindenma...@sap.com>; Sergey Bylokhov
<sergey.bylok...@oracle.com>; Vincent Ryan <vincent.x.r...@oracle.com>
Cc: awt-...@openjdk.java.net; 2d-dev <2d-dev@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 <sergey.bylok...@oracle.com>; Lindenmaier, Goetz
<goetz.lindenma...@sap.com>; Vincent Ryan <vincent.x.r...@oracle.com>
Cc: awt-...@openjdk.java.net; 2d-dev <2d-dev@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 <goetz.lindenma...@sap.com>
Cc: awt-...@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
<goetz.lindenma...@sap.com <mailto:goetz.lindenma...@sap.com> >

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

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

2016-12-01 Thread Lindenmaier, Goetz

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 <goetz.lindenma...@sap.com>; Sergey Bylokhov
> <sergey.bylok...@oracle.com>; Vincent Ryan <vincent.x.r...@oracle.com>
> Cc: awt-...@openjdk.java.net; 2d-dev <2d-dev@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 <sergey.bylok...@oracle.com>; Lindenmaier, Goetz
> >> <goetz.lindenma...@sap.com>; Vincent Ryan <vincent.x.r...@oracle.com>
> >> Cc: awt-...@openjdk.java.net; 2d-dev <2d-dev@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 <goetz.lindenma...@sap.com>
> >>>>> Cc: awt-...@openjdk.java.net; security-...@openjdk.java.net
> >>>>> Subject: Re: R

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

2016-12-01 Thread Phil Race


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 <sergey.bylok...@oracle.com>; Lindenmaier, Goetz
<goetz.lindenma...@sap.com>; Vincent Ryan <vincent.x.r...@oracle.com>
Cc: awt-...@openjdk.java.net; 2d-dev <2d-dev@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 <goetz.lindenma...@sap.com>
Cc: awt-...@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
<goetz.lindenma...@sap.com <mailto:goetz.lindenma...@sap.com> >

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 the 100 byte fixed-size string
statusWindow->status
by copying text->string.multi_byte without checking the length.

 gtk3_interface.c:

 This less-than-zero comparison of an unsigned value is never true.

 Using uninitialized value color. Field color.alpha is
uninitialized.
 E.g. used at gtk3_interface.c:2287.

 XToolkit.c

 Using uninitialized value ret_timeout.
 E.g. in XToolkit.c:6809.

 XWindow.c

 Argument is incompatible with corresponding format string
conversion.

 splashscreen_sys.c

 Overflowed or truncated value (or a value computed from an
overflowed or truncated value) (gdk_scale > 0) ? native_scale *
(double)gdk_scale : native_scale used as return value.

 ec.c

 Using uninitialized value k.dp when calling mp_clear.

 ecdecode.c

 You might overrun the 291 byte fixed-size string genenc by copying
curveParams->geny w

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

2016-12-01 Thread Lindenmaier, Goetz

Ah, no, that is just the field that is initialized with the 
Result of  gtk3_get_color_for_flags() why I need to initialize it there.

Best regards,
  Goetz.

> -Original Message-
> From: Lindenmaier, Goetz
> Sent: Donnerstag, 1. Dezember 2016 12:10
> To: 'Prasanta Sadhukhan' <prasanta.sadhuk...@oracle.com>; Phil Race
> <philip.r...@oracle.com>; Sergey Bylokhov <sergey.bylok...@oracle.com>;
> Vincent Ryan <vincent.x.r...@oracle.com>
> Cc: awt-...@openjdk.java.net; 2d-dev <2d-dev@openjdk.java.net>; security-
> d...@openjdk.java.net
> Subject: RE: [OpenJDK 2D-Dev]  RFR(M): 8170525: Fix minor issues
> in awt coding
> 
> Hi Prasanta,
> 
> good point, I added that too.
> 
> Best regards,
>   Goetz.
> 
> > -Original Message-
> > From: Prasanta Sadhukhan [mailto:prasanta.sadhuk...@oracle.com]
> > Sent: Donnerstag, 1. Dezember 2016 06:05
> > To: Phil Race <philip.r...@oracle.com>; Sergey Bylokhov
> > <sergey.bylok...@oracle.com>; Lindenmaier, Goetz
> > <goetz.lindenma...@sap.com>; Vincent Ryan <vincent.x.r...@oracle.com>
> > Cc: awt-...@openjdk.java.net; 2d-dev <2d-dev@openjdk.java.net>; security-
> > d...@openjdk.java.net
> > Subject: Re: [OpenJDK 2D-Dev]  RFR(M): 8170525: Fix minor
> issues
> > in awt coding
> >
> > Also, in gtk3_interface.c, there is this change for color.alpha
> >
> >
> > 2219 color.alpha = 0; in gtk3_get_color_for_flags()
> > but it is used in
> > gtk3_get_color_for_state() where it is not initialized
> >
> > 2268 GdkRGBA color;
> > Regards
> > Prasanta
> >
> > On 12/1/2016 1:28 AM, Phil Race wrote:
> >
> >
> > 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
> > <goetz.lindenma...@sap.com> <mailto:goetz.lindenma...@sap.com>
> > Cc: awt-...@openjdk.java.net <mailto:awt-
> > d...@openjdk.java.net> ; security-...@openjdk.java.net <mailto:security-
> > d...@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.
> >
> &g

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

2016-12-01 Thread Lindenmaier, Goetz

Hi Prasanta, 

good point, I added that too.

Best regards,
  Goetz.

> -Original Message-
> From: Prasanta Sadhukhan [mailto:prasanta.sadhuk...@oracle.com]
> Sent: Donnerstag, 1. Dezember 2016 06:05
> To: Phil Race <philip.r...@oracle.com>; Sergey Bylokhov
> <sergey.bylok...@oracle.com>; Lindenmaier, Goetz
> <goetz.lindenma...@sap.com>; Vincent Ryan <vincent.x.r...@oracle.com>
> Cc: awt-...@openjdk.java.net; 2d-dev <2d-dev@openjdk.java.net>; security-
> d...@openjdk.java.net
> Subject: Re: [OpenJDK 2D-Dev]  RFR(M): 8170525: Fix minor issues
> in awt coding
> 
> Also, in gtk3_interface.c, there is this change for color.alpha
> 
> 
> 2219 color.alpha = 0; in gtk3_get_color_for_flags()
> but it is used in
> gtk3_get_color_for_state() where it is not initialized
> 
> 2268 GdkRGBA color;
> Regards
> Prasanta
> 
> On 12/1/2016 1:28 AM, Phil Race wrote:
> 
> 
>   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
> <goetz.lindenma...@sap.com> <mailto:goetz.lindenma...@sap.com>
>   Cc: awt-...@openjdk.java.net <mailto:awt-
> d...@openjdk.java.net> ; security-...@openjdk.java.net <mailto:security-
> d...@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
>   <goetz.lindenma...@sap.com
> <mailto:goetz.lindenma...@sap.com>  <mailto:goetz.lindenma...@sap.com>
> <mailto:goetz.lindenma...@sap.com>  > 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 chan

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

2016-12-01 Thread Lindenmaier, Goetz

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 <sergey.bylok...@oracle.com>; Lindenmaier, Goetz
> <goetz.lindenma...@sap.com>; Vincent Ryan <vincent.x.r...@oracle.com>
> Cc: awt-...@openjdk.java.net; 2d-dev <2d-dev@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 <goetz.lindenma...@sap.com>
> >>> Cc: awt-...@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
> >>> <goetz.lindenma...@sap.com <mailto:goetz.lindenma...@sap.com> >
> 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:
> >>>
> >>

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

2016-12-01 Thread Lindenmaier, Goetz

Hi Christoph, 

I changed the code in fontpath.c to use snprintf, that seems much 
better as the concatenated string is a literal anyways.

Also, I fixed the spaces.

I'll post a new webrev once I'm through all the reviews.

Best regards,
  Goetz.

> -Original Message-
> From: Langer, Christoph
> Sent: Mittwoch, 30. November 2016 20:47
> To: Lindenmaier, Goetz 
> Cc: awt-...@openjdk.java.net; security-...@openjdk.java.net; 2d-
> d...@openjdk.java.net; Vincent Ryan 
> Subject: RE: RFR(M): 8170525: Fix minor issues in awt coding
> 
> Hi Goetz,
> 
> I have some small remarks.
> 
> src/java.desktop/unix/native/common/awt/fontpath.c:
> 
> 247 fontDirPath[sizeof(fontDirPath)-1] = '\0';
> -> you should add spaces left and right of '-'
> 248 strncat(fontDirPath, "/fonts.dir", sizeof(fontDirPath) -
> strlen(fontDirPath));
> -> I think you need to subtract 1 from the 3rd parameter of strncat ('size_t 
> n')
> because man page says:
> "If src contains n or more bytes, strncat() writes n+1 bytes to dest (n from 
> src
> plus the terminating null byte). Therefore, the size of dest must be at least
> strlen(dest)+n+1."
> 
> src/java.desktop/unix/native/libawt_xawt/awt/awt_InputMethod.c:
> 
> add spaces around '-' in the array index
> 
> The rest looks fine to me.
> 
> Best regards
> Christoph
> 
> > -Original Message-
> > From: security-dev [mailto:security-dev-boun...@openjdk.java.net] On
> Behalf
> > Of Lindenmaier, Goetz
> > Sent: Mittwoch, 30. November 2016 16:41
> > To: Vincent Ryan 
> > Cc: awt-...@openjdk.java.net; security-...@openjdk.java.net
> > Subject: RE: RFR(M): 8170525: Fix minor issues in awt coding
> >
> > 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-...@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 the 100 byte fixed-size string statusWindow->status
> > > by copying text->string.multi_byte without checking the length.
> > >
> > >   gtk3_interface.c:
> > >
> > >   This less-than-zero comparison of an unsigned value is never true.
> > >
> > >   Using uninitialized value color. Field color.alpha is uninitialized.
> > >   E.g. used at gtk3_interface.c:2287.
> > >
> > >   XToolkit.c
> > >
> > >   Using uninitialized value ret_timeout.
> > >   E.g. in XToolkit.c:6809.
> > >
> > >   XWindow.c
> > >
> > >   Argument is incompatible with corresponding format string conversion.
> > >
> > >   splashscreen_sys.c
> > >
> > >   Overflowed or truncated value (or a value computed from an
> > > overflowed or truncated value) (gdk_scale > 0) ? native_scale *
> > > (double)gdk_scale : native_scale used as return value.
> > >
> > >   ec.c
> > >
> > >   Using uninitialized value k.dp when calling mp_clear.
> > >
> > >   ecdecode.c
> > >
> > >   You might overrun the 291 byte fixed-size string genenc by copying
> > > curveParams->geny without checking the length.
> > >   Added sanity check before doing the string concatenation.
> > >
> > >   ecl_mult.c
> > >
> > >   Using uninitialized value kt.flag when

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

2016-11-30 Thread Semyon Sadetsky

yes, this is a potential issue. But actually cases MID,FOCUS,BLACK,WHITE 
are never used.


But the fix is wrong. It should be

color.alpha = 1; --Semyon


On 01.12.2016 08:05, Prasanta Sadhukhan wrote:


Also, in gtk3_interface.c, there is this change for color.alpha

2219 color.alpha = 0; in gtk3_get_color_for_flags()
but it is used in
gtk3_get_color_for_state() where it is not initialized

2268 GdkRGBA color;
Regards
Prasanta
On 12/1/2016 1:28 AM, Phil Race wrote:

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-...@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 the 100 byte fixed-size string 
statusWindow->status

by copying text->string.multi_byte without checking the length.

gtk3_interface.c:

This less-than-zero comparison of an unsigned value is never 
true.


Using uninitialized value color. Field color.alpha is 
uninitialized.

E.g. used at gtk3_interface.c:2287.

XToolkit.c

Using uninitialized value ret_timeout.
E.g. in XToolkit.c:6809.

XWindow.c

Argument is incompatible with corresponding format string 
conversion.


splashscreen_sys.c

Overflowed or truncated value (or a value computed from an
overflowed or truncated value) (gdk_scale > 0) ? native_scale *
(double)gdk_scale : native_scale used as return value.

ec.c

Using uninitialized value k.dp when calling mp_clear.

ecdecode.c

You might overrun the 291 byte fixed-size string genenc by 
copying

curveParams->geny without checking the length.
Added sanity check before doing the string concatenation.

ecl_mult.c

Using uninitialized value kt.flag when calling 
*group->point_mul. (The

function pointer resolves to ec_GF2m_pt_mul_mont.)

mpi.c

Using uninitialized value s. Field s.flag is uninitialized 
when calling

s_mp_exch.
Using uninitialized value tmp. Field tmp.flag is uninitialized 
when

calling s_mp_exch
Using uninitialized value t.dp when calling mp_clear.

p11_mutex.c

Using uninitialized value *ckpInitArgs. Field 
ckpInitArgs->flags is

uninitialized when calling memcpy.


DataBufferNative.c

Using uninitialized value lockInfo.rasBase when calling
BN_GetPixelPointer.

fontpath.c

You might overrun the 512 byte fixed-size string fontDirPath 
by copying

DirP->name[index] without checking the length.

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

2016-11-30 Thread Prasanta Sadhukhan


Also, in gtk3_interface.c, there is this change for color.alpha

2219 color.alpha = 0; in gtk3_get_color_for_flags()

but it is used in
gtk3_get_color_for_state() where it is not initialized

2268 GdkRGBA color;

Regards
Prasanta
On 12/1/2016 1:28 AM, Phil Race wrote:

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-...@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 the 100 byte fixed-size string 
statusWindow->status

by copying text->string.multi_byte without checking the length.

gtk3_interface.c:

This less-than-zero comparison of an unsigned value is never true.

Using uninitialized value color. Field color.alpha is 
uninitialized.

E.g. used at gtk3_interface.c:2287.

XToolkit.c

Using uninitialized value ret_timeout.
E.g. in XToolkit.c:6809.

XWindow.c

Argument is incompatible with corresponding format string 
conversion.


splashscreen_sys.c

Overflowed or truncated value (or a value computed from an
overflowed or truncated value) (gdk_scale > 0) ? native_scale *
(double)gdk_scale : native_scale used as return value.

ec.c

Using uninitialized value k.dp when calling mp_clear.

ecdecode.c

You might overrun the 291 byte fixed-size string genenc by copying
curveParams->geny without checking the length.
Added sanity check before doing the string concatenation.

ecl_mult.c

Using uninitialized value kt.flag when calling 
*group->point_mul. (The

function pointer resolves to ec_GF2m_pt_mul_mont.)

mpi.c

Using uninitialized value s. Field s.flag is uninitialized when 
calling

s_mp_exch.
Using uninitialized value tmp. Field tmp.flag is uninitialized 
when

calling s_mp_exch
Using uninitialized value t.dp when calling mp_clear.

p11_mutex.c

Using uninitialized value *ckpInitArgs. Field 
ckpInitArgs->flags is

uninitialized when calling memcpy.


DataBufferNative.c

Using uninitialized value lockInfo.rasBase when calling
BN_GetPixelPointer.

fontpath.c

You might overrun the 512 byte fixed-size string fontDirPath by 
copying

DirP->name[index] without checking the length.

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

2016-11-30 Thread Langer, Christoph

Hi Goetz,

I have some small remarks.

src/java.desktop/unix/native/common/awt/fontpath.c:

247 fontDirPath[sizeof(fontDirPath)-1] = '\0';
-> you should add spaces left and right of '-'
248 strncat(fontDirPath, "/fonts.dir", sizeof(fontDirPath) - 
strlen(fontDirPath));
-> I think you need to subtract 1 from the 3rd parameter of strncat ('size_t 
n') because man page says:
"If src contains n or more bytes, strncat() writes n+1 bytes to dest (n from 
src plus the terminating null byte). Therefore, the size of dest must be at 
least strlen(dest)+n+1."

src/java.desktop/unix/native/libawt_xawt/awt/awt_InputMethod.c:

add spaces around '-' in the array index

The rest looks fine to me.

Best regards
Christoph

> -Original Message-
> From: security-dev [mailto:security-dev-boun...@openjdk.java.net] On Behalf
> Of Lindenmaier, Goetz
> Sent: Mittwoch, 30. November 2016 16:41
> To: Vincent Ryan 
> Cc: awt-...@openjdk.java.net; security-...@openjdk.java.net
> Subject: RE: RFR(M): 8170525: Fix minor issues in awt coding
> 
> 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-...@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 the 100 byte fixed-size string statusWindow->status
> > by copying text->string.multi_byte without checking the length.
> >
> > gtk3_interface.c:
> >
> > This less-than-zero comparison of an unsigned value is never true.
> >
> > Using uninitialized value color. Field color.alpha is uninitialized.
> > E.g. used at gtk3_interface.c:2287.
> >
> > XToolkit.c
> >
> > Using uninitialized value ret_timeout.
> > E.g. in XToolkit.c:6809.
> >
> > XWindow.c
> >
> > Argument is incompatible with corresponding format string conversion.
> >
> > splashscreen_sys.c
> >
> > Overflowed or truncated value (or a value computed from an
> > overflowed or truncated value) (gdk_scale > 0) ? native_scale *
> > (double)gdk_scale : native_scale used as return value.
> >
> > ec.c
> >
> > Using uninitialized value k.dp when calling mp_clear.
> >
> > ecdecode.c
> >
> > You might overrun the 291 byte fixed-size string genenc by copying
> > curveParams->geny without checking the length.
> > Added sanity check before doing the string concatenation.
> >
> > ecl_mult.c
> >
> > Using uninitialized value kt.flag when calling *group->point_mul. (The
> > function pointer resolves to ec_GF2m_pt_mul_mont.)
> >
> > mpi.c
> >
> > Using uninitialized value s. Field s.flag is uninitialized when calling
> > s_mp_exch.
> > Using uninitialized value tmp. Field tmp.flag is uninitialized when
> > calling s_mp_exch
> > Using uninitialized value t.dp when calling mp_clear.
> >
> > p11_mutex.c
> >
> > Using uninitialized value *ckpInitArgs. Field ckpInitArgs->flags is
> > uninitialized when calling memcpy.
> >
> >
> > DataBufferNative.c
> >
> > Using uninitialized value lockInfo.rasBase when calling
> > BN_GetPixelPointer.
> >
> > fontpath.c
> >
> > You might overrun the 512 byte fixed-size string fontDirPath by copying
> > DirP->name[index] without checking the length.
> >

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

2016-11-30 Thread Sergey Bylokhov


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-...@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 the 100 byte fixed-size string statusWindow->status
by copying text->string.multi_byte without checking the length.

gtk3_interface.c:

This less-than-zero comparison of an unsigned value is never true.

Using uninitialized value color. Field color.alpha is uninitialized.
E.g. used at gtk3_interface.c:2287.

XToolkit.c

Using uninitialized value ret_timeout.
E.g. in XToolkit.c:6809.

XWindow.c

Argument is incompatible with corresponding format string conversion.

splashscreen_sys.c

Overflowed or truncated value (or a value computed from an
overflowed or truncated value) (gdk_scale > 0) ? native_scale *
(double)gdk_scale : native_scale used as return value.

ec.c

Using uninitialized value k.dp when calling mp_clear.

ecdecode.c

You might overrun the 291 byte fixed-size string genenc by copying
curveParams->geny without checking the length.
Added sanity check before doing the string concatenation.

ecl_mult.c

Using uninitialized value kt.flag when calling *group->point_mul. (The
function pointer resolves to ec_GF2m_pt_mul_mont.)

mpi.c

Using uninitialized value s. Field s.flag is uninitialized when calling
s_mp_exch.
Using uninitialized value tmp. Field tmp.flag is uninitialized when
calling s_mp_exch
Using uninitialized value t.dp when calling mp_clear.

p11_mutex.c

Using uninitialized value *ckpInitArgs. Field ckpInitArgs->flags is
uninitialized when calling memcpy.


DataBufferNative.c

Using uninitialized value lockInfo.rasBase when calling
BN_GetPixelPointer.

fontpath.c

You might overrun the 512 byte fixed-size string fontDirPath by copying
DirP->name[index] without checking the length.






--
Best regards, Sergey.

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

16 matches

Site Navigation

Mail list logo

Footer information