Segmentation fault while using const_cast to overload a constant function

Question

I'm having some problems with the const_cast function. I created a class Calorimeter that consists of a CaloGrid and some other stuff. I have to overload the grid() function to return the CaloGrid belonging to the class Calorimeter, however calling the main function returns a segmentation fault.

I know that using const_cast is not the best practice, but for this assignment I have to use it. Simply duplicating the code for the const CaloGrid& grid() const for the non-constant function would probably work.

What am I doing wrong? Is there a better way of doing this? And what is the point of overloading the function with a const copy of the function?

main.cpp

/*headers*/
int main() {
// create 2x2 Calorimeter object
Calorimeter C(2,2);
// return the CaloGrid class from this object
C.grid();
// gives segmentation error
}

Calorimeter.cpp

/*headers*/

// Calorimeter is object with CaloGrid of dimensions nx by ny
Calorimeter::Calorimeter(int nx,int ny){
// initalize the grid and allocate memory
    Cgrid = new CaloGrid(nx,ny);
}

Calorimeter::~Calorimeter(){
// delete memory
    delete Cgrid; 
}

// return the grid
const CaloGrid& Calorimeter::grid() const{
    return *Cgrid;
}

// segmentation error
CaloGrid& Calorimeter::grid(){
    return const_cast<CaloGrid&> (Calorimeter::grid());
}

Calorimeter.hh

#ifndef CALORIMETER_HH
#define CALORIMETER_HH

class Calorimeter {

public:    // Interface
    Calorimeter(int,int);

    ~Calorimeter();

    const CaloGrid& grid() const;

    CaloGrid& grid();

private:   // Implementation

CaloGrid *Cgrid;

}

#endif

I'd suspect you have been hitting undefined behavior. – πάντα ῥεῖ Dec 23 '16 at 14:33 — πάντα ῥεῖ, Dec 23 '16 at 14:33

score 4 · Answer 1 · answered Dec 23 '16 at 14:40

4

In

return const_cast<CaloGrid&> (Calorimeter::grid());

You are infinitly calling grid(). Since the grid function is non const Calorimeter::grid() will call the non const version of the function again, which calls the non const version again which, well, you get the point.

If you want to call the const version of the function then you need to cast this to const. You can do that with

const_cast<const Calorimeter&>(*this)

So with that your full code would look like

return const_cast<CaloGrid&>(const_cast<const Calorimeter&>(*this).grid());

If it doesn't look right it is probably the tears getting in your eyes from the code.

answered Dec 23 '16 at 14:40

NathanOliver

171,901
28
288
402

This did the trick. And I have tears in my eyes, the vagueness of these assignments is insanely frustrating. – Rooler Dec 23 '16 at 14:47
@Roeland: Did your teacher actually say *"use const_cast"* or did he/she say *"add a const overload of the function"*? Chances are you are completely misunderstanding the assignment, and `const_cast` is not the expected solution. – Christian Hackl Dec 23 '16 at 14:57
It said: "Add a function const CaloCell* cell(int x,int y) const. Optional: do you need to duplicate the code of the non-const cell() function? (Hint: think about using `const_cast<>` and `this`. – Rooler Dec 30 '16 at 13:27
@Rooler: This assignment is insanely well specified. And it's a good one, too, that actually tickles your brain enough to see, if you really understood what you are doing. Granted, though, if your non-const version can call into the const version, there's no need to even have a non-const version to begin with. – IInspectable Jan 01 '17 at 20:09

score 3 · Answer 2 · answered Dec 23 '16 at 14:37

Here's your class method:

CaloGrid& Calorimeter::grid(){

What does it do? Well:

return const_cast<CaloGrid&> (Calorimeter::grid());

It calls Calorimeter::grid(), and applies const_cast to its return value? What does this Calorimeter::grid() do? See above.

The issue of what const_cast does, and whether or not it's the right thing to do is irrelevant. This class method calls itself, resulting in infinite recursion, and your program blows quickly, as it runs out of its operating system-allotted stack space.

Although it's not quite clear what you're trying to do here, the answer as to the reason of your segfault is quite simple: infinite recursion.

The recursive call does not invoke the other, overloaded, const class method. This is being called from a mutable class method, so it picks the mutable overload, again.

Richard Hodges · Answer 3 · 2016-12-23T15:57:08.360

2

Expanding on the other posts, you may want to consider writing it this way:

#include <memory>

struct CaloGrid {
  CaloGrid(int x, int y) {};
};

class Calorimeter {

public:    // Interface
    Calorimeter(int,int);

    // no destructor - it's not necessary

    const CaloGrid& grid() const;

    CaloGrid& grid();

private:   // Implementation

  // resources managed automatically    
  std::unique_ptr<CaloGrid> Cgrid;
};

// Calorimeter is object with CaloGrid of dimensions nx by ny
Calorimeter::Calorimeter(int nx,int ny)
  : Cgrid { std::make_unique<CaloGrid>(nx, ny) }
{
}

// return the grid
const CaloGrid& Calorimeter::grid() const{
    return *Cgrid;
}

// no error any more
CaloGrid& Calorimeter::grid(){
    return *Cgrid;
}

int main() {
    // create 2x2 Calorimeter object
    // now we can use move-construction
    auto C = Calorimeter(2,2);

    // return the CaloGrid class from this object
    C.grid();
}

The raw pointer has been replaced with a smart pointer. This gives us (at least) 2 advantages:

Resource management is automated, so we can't forget to delete the CaloGrid, or accidentally delete it twice.
Calorimeter inherits the copy/move capabilities of the smart pointer (in this case, dangerous unwanted copies are disallowed but we get to keep moves and move-assignments)

Furthermore, although the grid method now repeats code, it repeats trivial code. The class has become much easier to both use correctly and maintain.

edited Dec 23 '16 at 15:57

answered Dec 23 '16 at 14:43

Richard Hodges

68,278
7
90
142

@LightnessRacesinOrbit it was in the original question. CaloGrid is a placeholder structure in this example - it wasn't supplied in the original code. – Richard Hodges Dec 23 '16 at 15:51
Yeah I realise that, just saying I don't see the point of it. At least you improved it with a smart pointer so it doesn't leak and can't break the class's copy semantics :) (insomuch as it prohibits the broken copies from compiling) I wonder whether this improvement and explanation is worth explicitly calling out in the answer? – Lightness Races in Orbit Dec 23 '16 at 15:51
@LightnessRacesinOrbit who knows - maybe it has a mutex in it? – Richard Hodges Dec 23 '16 at 15:52
Mm maybe. I would still delete the "local" copy constructor so as not to rely on inherited protections. But that may just be me – Lightness Races in Orbit Dec 23 '16 at 15:53
@LightnessRacesinOrbit I think that would be a reasonable improvement – Richard Hodges Dec 23 '16 at 15:53
*high-five* \o/ – Lightness Races in Orbit Dec 23 '16 at 15:53

score 0 · Answer 4 · answered Dec 23 '16 at 14:47

What you need is the following

CaloGrid & Calorimeter::grid()
{ 
    return const_cast<CaloGrid &>(const_cast<const Calorimeter *>(this)->grid());
                                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^                 
}

Otherwise the non-constant member function grid is recursively calls itself.

score 0 · Answer 5 · answered Dec 23 '16 at 14:51

I'm having some problems with the const_cast function.

It's an operator, not a function.

I know that using const_cast is not the best practice,

It is bad practice if you design your code's architecture such that const_cast cannot be avoided. It can be an acceptable real-life workaround if const correctness has already been violated elsewhere (e.g. in libraries you have to use, although well-designed libraries do not suffer from such shortcomings).

but for this assignment I have to use it.

Says who?

Simply duplicating the code for the const CaloGrid& grid() const for the non-constant function would probably work.

Yes, and it would be best practice. Just do it.

What am I doing wrong?

You've been a bit too eager to apply the DRY (Don't Repeat Yourself) principle. This has made you write code which has accidentally caused the infinite recursion problem explained by Sam in the other answer.

And what is the point of overloading the function with a const copy of the function?

The const overload allows you to call the function on a Calorimeter const object.

It's like std::vector::operator[], just to name a prominent example. Operations like that often come in const/non-const pairs.

You can look up how your compiler implements std::vector::operator[]. There's a good chance it will just duplicate the tiny piece of code necessary and not use const_cast or other tricks.

By the way...

It may not be a good idea to have a function like grid() in the first place. It practically makes the private data member public.

Segmentation fault while using const_cast to overload a constant function

5 Answers5