Log4cxx
  1. Log4cxx
  2. LOGCXX-234

Assignment operator removes const qualifier

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.10.0
    • Fix Version/s: 0.10.0
    • Component/s: None
    • Labels:
      None

      Description

      This overloaded assignment operator is not const-correct:
      ObjectPtrT& ObjectPtrT::operator=(const T* p1);
      This allows:
      typedef const Foo CFoo;
      CFoo* p = new CFoo(some_foo);
      ObjectPtrT<Foo> op;
      op = p;
      *op = another_foo;
      assert( *p == some_foo ); // boom!

      If there are cases where the const-ness needs to be removed, it should
      be done explicitly by the user of ObjectPtrT, since only the user
      knows if a particular case is safe or not. Doing it automatically
      inside ObjectPtrT hides potentially serious errors.
      I removed that assignment operator and successfully built and tested
      the lib with make check, so I conclude it's not needed anyway.

      If you do need to store a pointer-to-const in an ObjectPtrT you can do:
      ObjectPtrT<const Foo> op;
      op = p;

        Activity

        Hide
        Jonathan Wakely added a comment -

        this patch removes the unsafe and (apparently unused) assignment operator

        passes "make check" on x86-64/linux

        Show
        Jonathan Wakely added a comment - this patch removes the unsafe and (apparently unused) assignment operator passes "make check" on x86-64/linux
        Hide
        Curt Arnold added a comment -

        That function was just added in rev 613491 with the comment:

        "LOGCXX-219: Eliminate templated functions in ObjectPtrT template to fix VC6 build"

        I'll have to review it, but it seemed to be necessary 3 weeks ago to have the function for the VC6 build to work. It may be like operator< in that it is never intended to be called, but it needs to be present for STL collection exports to work properly.

        Show
        Curt Arnold added a comment - That function was just added in rev 613491 with the comment: " LOGCXX-219 : Eliminate templated functions in ObjectPtrT template to fix VC6 build" I'll have to review it, but it seemed to be necessary 3 weeks ago to have the function for the VC6 build to work. It may be like operator< in that it is never intended to be called, but it needs to be present for STL collection exports to work properly.
        Hide
        Jonathan Wakely added a comment -

        Eugh, I see. Sorry, I don't have any MS compilers to test with, especially not the extremely non-standard VC6 so I have no idea what it accepts.

        Show
        Jonathan Wakely added a comment - Eugh, I see. Sorry, I don't have any MS compilers to test with, especially not the extremely non-standard VC6 so I have no idea what it accepts.
        Hide
        Curt Arnold added a comment -

        I tried compiling the code with operator=(const T*) removed on VC 6, VC,NET and VC 2008 (sorry 2003 and 2005) and could not find the compile failure that hopefully motivated adding that assignment operator. However, there was a constructor that took const T* and I assume that it had something to do with keeping the assignment and constructors matched.

        I removed both and fixed up the few uses of the const T* constructor (in const methods where "this" was used to initialize a ObjectPtrT) in rev 627283.

        Show
        Curt Arnold added a comment - I tried compiling the code with operator=(const T*) removed on VC 6, VC,NET and VC 2008 (sorry 2003 and 2005) and could not find the compile failure that hopefully motivated adding that assignment operator. However, there was a constructor that took const T* and I assume that it had something to do with keeping the assignment and constructors matched. I removed both and fixed up the few uses of the const T* constructor (in const methods where "this" was used to initialize a ObjectPtrT) in rev 627283.

          People

          • Assignee:
            Curt Arnold
            Reporter:
            Jonathan Wakely
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development