[LOGCXX-202] ObjectPtrT has inconsistent const-ness on accessors

• Type: Bug
• Status: Resolved
• Priority: Major
• Resolution: Fixed
• Fix Version/s: 0.10.0
• Assignee: Curt Arnold
• Reporter: Curt Arnold
• Created: 08/Nov/07 22:13
• Updated: 08/Nov/07 22:44
• Resolved: 08/Nov/07 22:44

Description

On Nov 7, 2007, at 3:39 PM, Scott McCaskill wrote on log4cxx-user:

I've noticed the const version of this method is returning a pointer to const. The reason this is a problem is that it makes it impossible to log from const methods when LoggerPtr is a member variable:

class C
{
public:
C() : logger(log4cxx::Logger::getLogger("my logger")) { }

void constMethod() const

{ // Error: can't do this because the const version of ObjectPtrT::operator->() // must be used, and it returns a pointer to const T. LOG4CXX_DEBUG(logger, "debug message"); }

private:
log4cxx::LoggerPtr logger;
};

In both versions of operator->, the ObjectPtrT itself cannot be modified, so it's not clear why there is a need for a non-const version, or for a version that returns a pointer to const. It seems that in this case, the notion of pointer constness is being conflated with the notion of pointee constness.

Surely the above example is not such an unreasonable a thing to expect to be able to do?

BTW, I wasn't sure whether this was more appropriate for the user list or the dev list (I'm subscribed to both).

Scott McCaskill

---------

On Nov 8, 2007, at 1:40 PM, Scott McCaskill wrote on log4cxx-user:

....

Actually, I think there is a simpler solution. Here's what I tried:

===================================================================
— src/main/include/log4cxx/helpers/objectptr.h (revision 593214)
+++ src/main/include/log4cxx/helpers/objectptr.h (working copy)
@@ -137,8 +137,7 @@
bool operator!=(const ObjectPtrT& p1) const

{ return (this->p != p1.p); }

bool operator==(const T* p1) const

{ return (this->p == p1); }

bool operator!=(const T* p1) const

{ return (this->p != p1); }

• T* operator->() {return (T*) p; }
  - const T* operator->() const {return (const T*) p; }
  + T* operator->() const {return (T*) p; }

  T& operator*() const

  {return (T&) *p; }

  
  operator T*() const

  {return (T*) p; }

Note that the single version of operator-> that I added can be used anywhere that either of the previous versions could be used, as well as in the test case I originally described.

I tried this with msvc8. The trunk (593214) builds fine, as do the unit tests. The unit tests run as well as they did before. There is a crash midway through, but it crashes in the same place without any local changes. Naturally, all of my code is also fine with this change.

---------

Looking at the surrounding code, it seems inconsistent that the T& and T* operators are inconsistent in there const-ness. That the existing definitions would allow you to do:

(*logger).info("some message");

but not:

logger->info("some message");

Patch looks good to me.