Log4net
  1. Log4net
  2. LOG4NET-95

Level.CompareTo() may result a wrong Value -> sorting of Levels does not work

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.2.10
    • Fix Version/s: 1.2.11
    • Component/s: Core
    • Labels:
      None
    • Environment:
      VS2005 C#

      Description

      I want to show a sorted list of available Levels.
      The sort routine uses Level.Compare(Level l, Level r).
      The result might be wrong when comparing ALL to another Level, because there is an overflow when just subtracting the m_levelValue.

      try to call the integer compare method:
      Level.cs:
      public static int Compare(Level l, Level r)

      { ... //orig: return l.m_levelValue - r.m_levelValue; -> bug when int overflow return l.m_levelValue.CompareTo(r.m_levelValue); }

      hope this helps.

      best regards, Bernd.

        Activity

        Hide
        Ron Grabowski added a comment -

        The CompareTo fix seems ok. I suspect subtraction was a clever way to save some cpu cycles.

        FYI, Mono uses < and > instead of subtraction:

        http://svn.myrealbox.com/viewcvs/*checkout*/trunk/mcs/class/corlib/System/Int32.cs

        int xv = (int) v;
        if (m_value == xv)
        return 0;
        if (m_value > xv)
        return 1;
        else
        return -1;

        Another solution would be to add a comment to the Level constructor and throw an ArgumentException for level values less than zero. There are 10,000 positive numbers between 0 and the lowest built-in log4net Level.

        Since we know the two items we're comparing are ints, I think the inlined if/if/else block might save some cycles over CompareTo.

        Show
        Ron Grabowski added a comment - The CompareTo fix seems ok. I suspect subtraction was a clever way to save some cpu cycles. FYI, Mono uses < and > instead of subtraction: http://svn.myrealbox.com/viewcvs/*checkout*/trunk/mcs/class/corlib/System/Int32.cs int xv = (int) v; if (m_value == xv) return 0; if (m_value > xv) return 1; else return -1; Another solution would be to add a comment to the Level constructor and throw an ArgumentException for level values less than zero. There are 10,000 positive numbers between 0 and the lowest built-in log4net Level. Since we know the two items we're comparing are ints, I think the inlined if/if/else block might save some cycles over CompareTo.
        Hide
        Ron Grabowski added a comment -

        Fixed in 463328. Replaced subtraction operator with System.Int32's CompareTo to avoid possible overflow errors. All tests continue to pass.

        Show
        Ron Grabowski added a comment - Fixed in 463328. Replaced subtraction operator with System.Int32's CompareTo to avoid possible overflow errors. All tests continue to pass.

          People

          • Assignee:
            Ron Grabowski
            Reporter:
            Bernd Klaiber
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development