Bug 58535

Summary: ReverseComparator unsafely negates result
Product: Tomcat 8 Reporter: Anthony Whitford <anthony>
Component: CatalinaAssignee: Tomcat Developers Mailing List <dev>
Status: RESOLVED FIXED    
Severity: normal    
Priority: P2    
Version: 8.0.x-trunk   
Target Milestone: ----   
Hardware: PC   
OS: Mac OS X 10.1   
Attachments: Patch to avoid negation risk

Description Anthony Whitford 2015-10-25 19:55:18 UTC
Created attachment 33210 [details]
Patch to avoid negation risk

Consider the code:

    @Override
    public int compare(Session o1, Session o2) {
        int returnValue = comparator.compare(o1, o2);
        return (- returnValue);
    }

This code negates the return value of the compare method. This is a questionable or bad programming practice, since if the return value is Integer.MIN_VALUE, negating the return value won't negate the sign of the result. You can achieve the same intended result by reversing the order of the operands rather than by negating the results.

See http://findbugs.sourceforge.net/bugDescriptions.html#RV_NEGATING_RESULT_OF_COMPARETO

Recommend the following:

    @Override
    public int compare(Session o1, Session o2) {
        // Note that comparing o2 with o1 to get reverse result...
        return comparator.compare(o2, o1);
    }
Comment 1 Violeta Georgieva 2015-10-28 13:41:25 UTC
Hi,

Thanks for the report and the patch.
I decided to remove this class and to use the standard Collections.reverseOrder(Comparator) method.
The fix is available in trunk, 8.0.x (for 8.0.29 onwards) and 7.0.x (for 7.0.66
onwards)

Regards,
Violeta