<Swing Dev> Suspicious field assignment in StrikeMetrics

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

<Swing Dev> Suspicious field assignment in StrikeMetrics

Andrey Turbanov
Hello.
I've found suspicious code in constructor sun.font.StrikeMetrics#StrikeMetrics()

StrikeMetrics() {
    ascentX = ascentY = Integer.MAX_VALUE;
    descentX = descentY = leadingX = leadingY = Integer.MIN_VALUE;
    baselineX = baselineX = maxAdvanceX = maxAdvanceY = Integer.MIN_VALUE;
}

As you can see, basLineX field is assigned twice.
Looks like it's supposed to be like this:

baselineX = baselineY = maxAdvanceX = maxAdvanceY = Integer.MIN_VALUE;



Found by SpotBugs
https://spotbugs.readthedocs.io/en/stable/bugDescriptions.html#sa-double-assignment-of-field-sa-field-double-assignment

Andrey Turbanov
Reply | Threaded
Open this post in threaded view
|

Re: <Swing Dev> Suspicious field assignment in StrikeMetrics

Volodin, Vladislav
Hello Andrey,

Nice catch! You can create a bug and prepare a pull request, so maintainers can review it.
What I have just personally noticed, all fields have the type float, so probably instead of "Integer.MIN_VALUE", I would recommend to use "-Float.MAX_VALUE". But since this code is about 13-years old, I don't know if reviewers can accept your PR without additional tests.

Kind regards,
Vlad

-----Original Message-----
From: swing-dev <[hidden email]> On Behalf Of Andrey Turbanov
Sent: Dienstag, 5. Januar 2021 12:40
To: [hidden email]
Subject: <Swing Dev> Suspicious field assignment in StrikeMetrics

Hello.
I've found suspicious code in constructor sun.font.StrikeMetrics#StrikeMetrics()

StrikeMetrics() {
    ascentX = ascentY = Integer.MAX_VALUE;
    descentX = descentY = leadingX = leadingY = Integer.MIN_VALUE;
    baselineX = baselineX = maxAdvanceX = maxAdvanceY = Integer.MIN_VALUE;
}

As you can see, basLineX field is assigned twice.
Looks like it's supposed to be like this:

baselineX = baselineY = maxAdvanceX = maxAdvanceY = Integer.MIN_VALUE;



Found by SpotBugs
https://spotbugs.readthedocs.io/en/stable/bugDescriptions.html#sa-double-assignment-of-field-sa-field-double-assignment

Andrey Turbanov
Reply | Threaded
Open this post in threaded view
|

Re: <Swing Dev> Suspicious field assignment in StrikeMetrics

Andrey Turbanov
I don't have a JBS account yet, so I can't create a bug.
I hope someone, who read it can create it.

Andrey Turbanov

вт, 5 янв. 2021 г. в 15:14, Volodin, Vladislav <[hidden email]>:

>
> Hello Andrey,
>
> Nice catch! You can create a bug and prepare a pull request, so maintainers can review it.
> What I have just personally noticed, all fields have the type float, so probably instead of "Integer.MIN_VALUE", I would recommend to use "-Float.MAX_VALUE". But since this code is about 13-years old, I don't know if reviewers can accept your PR without additional tests.
>
> Kind regards,
> Vlad
>
> -----Original Message-----
> From: swing-dev <[hidden email]> On Behalf Of Andrey Turbanov
> Sent: Dienstag, 5. Januar 2021 12:40
> To: [hidden email]
> Subject: <Swing Dev> Suspicious field assignment in StrikeMetrics
>
> Hello.
> I've found suspicious code in constructor sun.font.StrikeMetrics#StrikeMetrics()
>
> StrikeMetrics() {
>     ascentX = ascentY = Integer.MAX_VALUE;
>     descentX = descentY = leadingX = leadingY = Integer.MIN_VALUE;
>     baselineX = baselineX = maxAdvanceX = maxAdvanceY = Integer.MIN_VALUE;
> }
>
> As you can see, basLineX field is assigned twice.
> Looks like it's supposed to be like this:
>
> baselineX = baselineY = maxAdvanceX = maxAdvanceY = Integer.MIN_VALUE;
>
>
>
> Found by SpotBugs
> https://spotbugs.readthedocs.io/en/stable/bugDescriptions.html#sa-double-assignment-of-field-sa-field-double-assignment
>
> Andrey Turbanov
Reply | Threaded
Open this post in threaded view
|

Re: <Swing Dev> Suspicious field assignment in StrikeMetrics

Philip Race

Raising this on the right (2d) list would have helped.
https://bugs.openjdk.java.net/browse/JDK-8263124

-phil.

On 1/5/21 6:23 AM, Andrey Turbanov wrote:

> I don't have a JBS account yet, so I can't create a bug.
> I hope someone, who read it can create it.
>
> Andrey Turbanov
>
> вт, 5 янв. 2021 г. в 15:14, Volodin, Vladislav <[hidden email]>:
>> Hello Andrey,
>>
>> Nice catch! You can create a bug and prepare a pull request, so maintainers can review it.
>> What I have just personally noticed, all fields have the type float, so probably instead of "Integer.MIN_VALUE", I would recommend to use "-Float.MAX_VALUE". But since this code is about 13-years old, I don't know if reviewers can accept your PR without additional tests.
>>
>> Kind regards,
>> Vlad
>>
>> -----Original Message-----
>> From: swing-dev <[hidden email]> On Behalf Of Andrey Turbanov
>> Sent: Dienstag, 5. Januar 2021 12:40
>> To: [hidden email]
>> Subject: <Swing Dev> Suspicious field assignment in StrikeMetrics
>>
>> Hello.
>> I've found suspicious code in constructor sun.font.StrikeMetrics#StrikeMetrics()
>>
>> StrikeMetrics() {
>>      ascentX = ascentY = Integer.MAX_VALUE;
>>      descentX = descentY = leadingX = leadingY = Integer.MIN_VALUE;
>>      baselineX = baselineX = maxAdvanceX = maxAdvanceY = Integer.MIN_VALUE;
>> }
>>
>> As you can see, basLineX field is assigned twice.
>> Looks like it's supposed to be like this:
>>
>> baselineX = baselineY = maxAdvanceX = maxAdvanceY = Integer.MIN_VALUE;
>>
>>
>>
>> Found by SpotBugs
>> https://spotbugs.readthedocs.io/en/stable/bugDescriptions.html#sa-double-assignment-of-field-sa-field-double-assignment
>>
>> Andrey Turbanov