Details

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

      Description

      There are a number of casts to and from void* in ObjectPtrT which could be simplified by introducing a new member function to abstract away the casts so that they are only done in one place. The uses of ObjectPtrBase::exchange can be changed to the new function and so don't need any casts. To avoid any accidental ambiguity, the original ObjectPtrBase::exchange could be renamed (user code outside the library that uses that function directly would have to be changed to call the new typesafe function, or call the renamed original function)

        Activity

        Hide
        Jonathan Wakely added a comment -

        This patch implements the suggestion. Passes "make check" on x86_64/linux.

        This assumes the patch for LOGCXX-234 has been applied, as it doesn't adjust the assignment operator removed by the other patch.

        If the patch for LOGCXX-234 is not applied, this extra change will be needed:

        • void* oldPtr = ObjectPtrBase::exchange((void*) pp, const_cast<T>(p1));
          + void* oldPtr = ObjectPtrBase::exchangeVoidP((void*) pp, const_cast<T>(p1));
        Show
        Jonathan Wakely added a comment - This patch implements the suggestion. Passes "make check" on x86_64/linux. This assumes the patch for LOGCXX-234 has been applied, as it doesn't adjust the assignment operator removed by the other patch. If the patch for LOGCXX-234 is not applied, this extra change will be needed: void* oldPtr = ObjectPtrBase::exchange((void* ) pp, const_cast<T >(p1)); + void* oldPtr = ObjectPtrBase::exchangeVoidP((void* ) pp, const_cast<T >(p1));
        Hide
        Jonathan Wakely added a comment -

        actually, ignore the end of the previous comment ... you'd want to actually change the function to use the new exchange() ... what I showed was what I used to make it compile when I tested, but you'd want to fix it properly.

        I suggest applying the patch for LOGCXX-234 instead of course

        Show
        Jonathan Wakely added a comment - actually, ignore the end of the previous comment ... you'd want to actually change the function to use the new exchange() ... what I showed was what I used to make it compile when I tested, but you'd want to fix it properly. I suggest applying the patch for LOGCXX-234 instead of course
        Hide
        Curt Arnold added a comment -

        Committed a different fix in 627223. The original fix was missing the corresponding change in the ObjectPtrBase implementation.

        Anyway since the first argument was always the pointer member, it was just easier to drop that parameter and make exchange a member function which eliminated the collision with the static ObjectPtrBase::exchange.

        Show
        Curt Arnold added a comment - Committed a different fix in 627223. The original fix was missing the corresponding change in the ObjectPtrBase implementation. Anyway since the first argument was always the pointer member, it was just easier to drop that parameter and make exchange a member function which eliminated the collision with the static ObjectPtrBase::exchange.

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development