0

I have the following structure:

struct CountCarrier
{
    int *CurrCount;
};

And this is what I want to do:

int main()
{
    CountCarrier carrier = CountCarrier();
    *(carrier.CurrCount) = 2;  // initialize the *(carrier.CurrCount) to 2
    IncreaseCount(&carrier);  // should increase the *(carrier.CurrCount) to 3
}


void IncreaseCount(CountCarrier *countCarrier)
{
    int *currCounts = countCarrier->CurrCount;
    (*currCounts)++;
}

So, my intention is specified in the comments.

However, I couldn't get this to work. For starters, the program throws an exception at this line:

*(carrier.CurrCount) = 2;

And I suspect the following line won't work as well. Anything I did wrong?

iammilind
  • 68,093
  • 33
  • 169
  • 336
Graviton
  • 81,782
  • 146
  • 424
  • 602
  • 4
    It throws an exception because `CurrCount` doesn't point to a valid memory region. But that's not even the biggest problem with the code. [Go pick up a good introductory C++ book](http://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list) and read it. – In silico Oct 05 '11 at 03:54
  • @Keith: You're confusing `*(carrier.CurrCount)` with `(*carrier).currCount`. Only the latter is equivalent to `carrier->CurrCount`. – Idelic Oct 05 '11 at 04:41
  • @Idelic: D'oh! You're right. I've deleted the comment. – Keith Thompson Oct 05 '11 at 05:02

6 Answers6

4
    struct CountCarrier 
    {     
        int *CurrCount;  //No memory assigned
    }; 

You need to allocate some valid memory to the pointer inside the structure to be able to put data in this.

Unless you do so, What you ar trying to do is attempting to write at some invalid address, which results in an Undefined Behavior, which luckiy in this case shows up as an exception.

Resolution:

    struct CountCarrier 
    {     
        int *CurrCount;  //No memory assigned
        CountCarrier():CurrCount(new(int))
        {

        }
    }; 

Suggestion:
Stay away from dynamic allocations as long as you can.
When you think of using pointers always think whether you really need one. In this case it doesn't really seem that you need one, A simple int member would be just fine.

Alok Save
  • 202,538
  • 53
  • 430
  • 533
1

You need to create the pointer. ie. carrier->CurrCount = new int;

Ed Heal
  • 59,252
  • 17
  • 87
  • 127
  • I would also add: If you're doing allocation sizes so small (`new int`), I have to question whether it's worth making this a separate allocation at all. In other words, why not just put an integer in the struct. (Also, since the questioner is a beginner, he should be steered towards `delete` and smart pointers...) – asveikau Oct 05 '11 at 04:00
0
*(carrier.CurrCount)

This is dereferencing the pointer carrier.CurrCount, but you never initialized it. I suspect this is what you want:

carrier.CurrCount = new int(2);
Brendan Long
  • 53,280
  • 21
  • 146
  • 188
0

I seriously doubt that your program throws an exception at the line:

*(carrier.CurrCount) = 2;

While throwing an exception is certainly allowed behaviour, it seems much more likely that you encountered an access violation that caused the process to be killed by the operating system.

The problem is that you are using a pointer, but your pointer is not initialised to point at anything. This means that the result of the pointer dereference is undefined.

In this situation there does not seem to be any advantage to using a pointer at all. Your CurrCount member would work just as well if it was just a plain int.

Mankarse
  • 39,818
  • 11
  • 97
  • 141
0

As Als said, you need to provide some memory for the code to work.

But why make it so complicated? You don't need any pointers for the code you have to work. The "modern C++" way looks more like this:

struct CountCarrier 
{ 
public:
    CountCarrier(int currCount) : currCount(currCount) {}
    void IncreaseCount() { ++currCount; }
    int GetCount() const { return currCount; }
private:
    int currCount; 
}; 

int main() 
{ 
    CountCarrier carrier(2); // Initialize carrier.currCount to 2 
    carrier.IncreaseCount();  // Increment carrier.currCount to 3
}

Note how much cleaner and less error prone that is. Like I said, pick up a good introductory C++ book and read through it.

Community
  • 1
  • 1
In silico
  • 51,091
  • 10
  • 150
  • 143
  • This is a scale down from my real problem; I am trying to check whether I am using the pointer correctly or not. – Graviton Oct 05 '11 at 04:04
  • @Graviton: My point still stands. Pointers are useful (and necessary!), but there are lots of times where it's not needed. A good C++ book will cover proper pointer usage, so I still recommend a C++ book. – In silico Oct 05 '11 at 04:08
  • Oh, I forgot to mention another point; `CountCarrier` would be exposed to .Net code as the old `dllexport` mecahinism, Shouldn't I use pointer for `currCount` to get it to work? The actual type of `currCount` is very complicated and I don't really know how to represent it in C# interop. – Graviton Oct 05 '11 at 04:11
  • @Graviton: See, that's the kind of detail that would benefit everybody here if you put that in your question in the first place. The .Net aspect of it is significant. – In silico Oct 05 '11 at 04:13
  • Oh no, I think I'll just accept current answer and file another question. – Graviton Oct 05 '11 at 04:30
0

If you are using C++, then you should encash its facilities. Instead of correcting your code, I am showing here that how the code should look like:

struct CountCarrier
{
  int CurrCount; // simple data member

  CountCarrier(int count) : CurrCount(count) {} // constructor

  CountCarrier& operator ++ ()  // overloaded operator
  {
    ++ CurrCount;
    return *this;
  }
};

We are overloading operator ++, because you have only one data member. You can replace with some named method also, like void IncrementCount().

CountCarrier carrier(2);
++ carrier;
iammilind
  • 68,093
  • 33
  • 169
  • 336