CODEWITHSUNDEEP
c++classpointersreferencemember
Agrim Pathak
Agrim Pathak

Reputation: 3207

Returning a reference to an element of a class member container

Foo behaves like a circular iterator. Despite me being nervous about it, the code below compiles fine, but creates a run-time error. I receive the error even if I remove the consts from get_current(). Of course, I can return a pointer and it'll work; however, will I get better security returning a reference?

#include <iostream>
#include <array>
#include <memory>

class Foo
{
public:
  Foo();
  void next();
  const int& get_current() const;
private:
  std::array<std::unique_ptr<int>, 3> arr_;
  unsigned i_;
};

Foo::Foo() : i_(0)
{
  arr_[0] = std::unique_ptr<int>(new int(5));
  arr_[1] = std::unique_ptr<int>(new int(6));
  arr_[2] = std::unique_ptr<int>(new int(7));
}

void Foo::next()
{
  ++i_;
  i_ %= 3;
}

const int& Foo::get_current() const 
{
  return *arr_[i_];
}

int main()
{
  Foo foo;
  int* p;

  *p = foo.get_current();
  //do something with p
  std::cout << *p << std::endl;

  foo.next();
  *p = foo.get_current();
  //do something with p
  std::cout << *p << std::endl;

  return 0;
}

Upvotes: 2

Views: 121

Answers (3)

Bathsheba
Bathsheba

Reputation: 234785

foo.get_current(); may well be returning a const reference, but after that you're attempting to take a value copy of that when assigning to *p.

Assigning to *p is what's causing you trouble as p is uninitialised. That's undefined behaviour and is manifesting itself in your case as a runtime error.

You could use code like const int& p = foo.get_current(); but do be aware that a reference can only be bound once, so you'll have to be careful with scoping.

Or, you could use std::shared_ptr<int> and make that the return type of get_current(), and strip your code entirely of bare pointers.

Upvotes: 2

Mike Seymour
Mike Seymour

Reputation: 254631

int* p;

That's an uninitialised pointer, not pointing to anything. Dereferencing it gives undefined behaviour.

*p = foo.get_current();

That dereferences the invalid pointer. Boom!

Perhaps you want it to point to the array element

p = &foo.get_current();

or perhaps you want a copy of the array element

int n;
n = foo.get_current();

Upvotes: 2

πάντα ῥεῖ
πάντα ῥεῖ

Reputation: 1

*p = ... You dereference int* P without having it properly initialized.

Change your code in main to 

 int p; // Remove *

 p = foo.get_current();
 //do something with p
 std::cout << p << std::endl;

or if you really meant to use a pointer

 const int* p;

 p = &foo.get_current();
  // ^ Take the address

Upvotes: 1

Related Questions