6

I am compiling Percona (MySQL variant) on my Raspberry Pi, which has an ARM processor.

I've encountered an issue during compilation that has been reported, but nobody is willing to fix because ARM is an unsupported platform.

https://bugs.launchpad.net/percona-server/+bug/1002848

I've managed to work around the issue and successfully compile, but my knowledge of c++ is somewhat lacking and I don't fully understand if I have actually broken something or not.

I've read through lots of the invalid conversion from const char* to char* questions on SO, which is where I got the idea for this workaround.

The error was as follows:

error: invalid conversion from 'const pthread_mutex_t*' to 'pthread_mutex_t*'

(it actually wasn't pthread_mutex_t on my platform, but the issue is the same - actual type lost to the infinite abyss that is the scrollback buffer)

The offending code was:

  uint32 count(uint index) const
  {
    my_atomic_rwlock_rdlock(&time_collector_lock);

I changed this to:

  uint32 count(uint index) const
  {
    my_atomic_rwlock_t dummy = time_collector_lock;
    my_atomic_rwlock_rdlock(&dummy);

time_collector_lock is defined as:

private:
  my_atomic_rwlock_t time_collector_lock;

Due to the fact this is supposed to be a mutex, I have a feeling I've probably made this non-thread-safe. Or is this ok?

Is there a better solution?

Leigh
  • 12,859
  • 3
  • 39
  • 60
  • Why do not use `const_cast`? You'll make your **intent clear** and you won't create a **copy** of pthread_mutex_t... – Adriano Repetti Jul 16 '12 at 09:57
  • If you try to `const_cast` an object that was initially declared as `const`, the result is UB – SingerOfTheFall Jul 16 '12 at 10:01
  • @SingerOfTheFall The **only** purpose of const_cast is to strip out the const. – Adriano Repetti Jul 16 '12 at 10:02
  • @Adriano, [msdn](http://msdn.microsoft.com/en-us/library/bz6at95h%28v=vs.80%29.aspx): For pointers to data members, the result will refer to the same member as the original (uncast) pointer to data member. Depending on the type of the referenced object, a write operation through the resulting pointer, reference, or pointer to data member might produce undefined behavior. You cannot use the const_cast operator to directly override a constant variable's constant status. – SingerOfTheFall Jul 16 '12 at 10:05
  • @Adriano, also, [this](http://stackoverflow.com/questions/357600/is-const-cast-safe) – SingerOfTheFall Jul 16 '12 at 10:07
  • @SingerOfTheFall re-read the 2nd link you posted. Here we have a pointer to non-const data member. – Adriano Repetti Jul 16 '12 at 10:14
  • 1
    @Adriano how do you know it's non-const? `class_that_has_said_data_member const boom;`. Don't bother with `const_cast` if you want something to be mutable (which is desired here). *That's not what it is meant for*. It exists to interact with bad APIs that don't modify parameters, and yet don't advertise that. It's to be used when you still want immutability all along. `mutable` exists to make things mutable. – R. Martinho Fernandes Jul 16 '12 at 10:15
  • @R.MartinhoFernandes I know it's non-const because declaration is in the question text. I agree about all the stuff about mutable vs const_cast (=const_cast for legacy code, mutable for anything else). What I do not agree is to use mutable for locks, they **do** change the state of the object and they're not an implementation detail (like a cache). I would avoid mutable for locks (because any multi-threading related issue isn't something I want to hide) and I **may relax** this for few methods with a const_cast (if REALLY needed). – Adriano Repetti Jul 16 '12 at 10:47
  • @R.MartinhoFernandes anyway I see this is an endless debate (to use or not mutable for locks)... – Adriano Repetti Jul 16 '12 at 10:49
  • @Adriano *No, the declaration is not in the question text*. You have the declaration of the member, but not of the object itself. If that declaration is `class_that_has_said_data_member const boom;`, the data member is `const`, regardless of its own declaration. Using `const_cast` in that situation yields undefined behaviour. – R. Martinho Fernandes Jul 16 '12 at 10:49
  • @R.MartinhoFernandes if the object itself is const then you do not need any synchronization at all because it won't ever be changed... – Adriano Repetti Jul 16 '12 at 10:52
  • @Adriano When you're implementing it, you can't assume it will always be const, just like you can't assume it will always be non-const. So, you can't implement it without synchronization because it needs to work when it's non-const. You also can't do it with `const_cast` because it needs to work when it's const. See the problem now? `mutable` exists for this. Why shun *the* solution? – R. Martinho Fernandes Jul 16 '12 at 10:54
  • @R.MartinhoFernandes For this I wouldn't use const at all because of locks (see previous comment) but you're right, in this case mutable _is_ the solution! – Adriano Repetti Jul 16 '12 at 10:59

1 Answers1

10

It seems, in the class, you have declared the member data as:

pthread_mutex_t time_collector_lock;

so in the const member function, this member data becomes as if you've declared it as:

const pthread_mutex_t time_collector_lock; //(as-if declaration)

which is causing the problem, as you cannot pass pointer to const object to my_atomic_rwlock_rdlock() which is expecting pointer to non-const object.

The keyword mutable can save you here. Declare the member data as mutable object as:

 mutable pthread_mutex_t time_collector_lock;
//^^^^^^note this

Now you can use the member data in const member function as well:

uint32 count(uint index) const
{
   my_atomic_rwlock_rdlock(&time_collector_lock);  //ok now!
Nawaz
  • 353,942
  • 115
  • 666
  • 851
  • 2
    I have this pattern. Locks and such are not part of the logical state of an object (indeed, their use is to preserve its logical state), so should be mutable where necessary. – Kaz Dragon Jul 16 '12 at 10:02
  • 3
    It is actually defined as `my_atomic_rwlock_t time_collector_lock;` adding `mutable` did the trick. Many thanks! – Leigh Jul 16 '12 at 10:11