Segmentation fault - reading array of initialized pointers

Question

could you help me with little problem?

I have following class;

class Link
{
  private:
    Demand *demand_[NUMBER_OF_CORES][NUMBER_OF_SLICES];

  public:
    Link()
    {
      for (int i = 0; i < NUMBER_OF_CORES; i++)
      {
        for (int j = 0; j < NUMBER_OF_SLICES; j++)
        {
          demand_[i][j] = NULL;
        }
      }
    }

    int virtualPut();
}

There will be problem with demand_ array. In the constructor everything is fine, after initialization I can use if (demand_[i][j] == NULL).

Problem starts in virtualPut()

int Link::virtualPut()
{
  for (int i = 0; i < NUMBER_OF_CORES; i++)
  {
    for (int j = 0; j < NUMBER_OF_SLICES; j++)
    {
      std::cout << "We're in " << i << " " << j << " \n" << std::flush;

      if (demand_[i][j] == NULL)  //SEGMENTATION FAULT
      {
        std::cout << "EMPTY\n";
      }
    }
  }
}

And also - if I call virtualPut() in constructor (just for test) it works fine.

But outside Link class I use.

void someFunction(Link *tab, int links)
{
  tab = new Link[links];

  tab[0].virtualPut();  //also just for test
}

What could be a problem here? I know that I can use vector, but that won't help me understand this memory problem.

One more thing - Dr. Memory says:

UNADDRESSABLE ACCESS: reading 0x0000000000000009-0x0000000000000011 8 byte(s)

But why?

EDIT! Problem solved in comments, thank you

Where do you initialize demand_ pointer? From your code it's uninitialized — Alexey Andronov
– Alexey Andronov, Commented Oct 23, 2018 at 9:21
But I declared it in static way - shouldn't compilator reserve memory for whole Demand *demand_[NUMBER_OF_CORES][NUMBER_OF_SLICES];? — mrozo
– mrozo, Commented Oct 23, 2018 at 9:25
In constructor I initialize whole array as NULL. Cannot I then check if demand_[x][y] is NULL or not? — mrozo
– mrozo, Commented Oct 23, 2018 at 9:28
Within someFunction, tab is a Link*. Try tab = new Link(); tab.virtualPut(); — Robert Kock
– Robert Kock, Commented Oct 23, 2018 at 9:34
@mrozo Your someFunction is strange. You're passing a pointer to Link as the first argument, changing it within the function, but the caller to someFunction will never receive those changes due to the pointer being passed by value. Methinks there are other errors in your code that you're not showing us that is causing the issue. Please post a minimal reproducible example. Also, since Link is a class, it requires that the Link object itself be valid for anything to work properly, including the demand_ member. We have no idea if Link is valid or not. — PaulMcKenzie
– PaulMcKenzie, Commented Oct 23, 2018 at 9:41

mgueydan · Accepted Answer · 2018-10-23 11:58:26Z

0

The code you show us is okay. I ran it on my side with huge values, and there is no Segfault. You declared a "Demand* array of array" in your Link class, and this is a valid declaration, the memory should be allocated.

What I do suspect is that NUMBER_OF_CORES and/or NUMBER_OF_SLICES doesn't have the same value in the code where you defines the Link class and in the code where you defined the virtualPut method.

Something like :

#define NUMBER_OF_CORES 10
#define NUMBER_OF_SLICES 10
class Link
{
private:
    Demand *demand_[NUMBER_OF_CORES][NUMBER_OF_SLICES];
...
}

and

#define NUMBER_OF_CORES 5000
#define NUMBER_OF_SLICES 5000
int Link::virtualPut()
{
    for (int i = 0; i < NUMBER_OF_CORES; i++)
    {
        for (int j = 0; j < NUMBER_OF_SLICES; j++)
        {
        // here you will have buffer overflow
    ...
}

What I would do :

use std::vector
probably use a single entry array, and wrap it up
don't use #define, it's messy
don't use arrays, it generates buffer overflow

That would be something like this :

class Link
{
private:
    std::vector<Demand*> demand_;
    const int NUMBER_OF_CORES = 10;
    const int NUMBER_OF_SLICES = 50;
private:
    int getIdx(int i, int j)
    {
        return i*NUMBER_OF_SLICES + j;
    }
public:
    Link()
    {
        demand_.resize(NUMBER_OF_CORES * NUMBER_OF_SLICES);
        for (int i = 0; i < NUMBER_OF_CORES; i++)
        {
            for (int j = 0; j < NUMBER_OF_SLICES; j++)
            {
                demand_[getIdx(i,j)] = NULL;
            }
        }
    }
    int virtualPut();
};

Note : additionaly, you showed us a virtualPut() that should return an int but doesn't.

edited Oct 23, 2018 at 11:58

answered Oct 23, 2018 at 11:52

mgueydan

3994 silver badges14 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

mrozo Over a year ago

I'm not sure using vector is a good idea. I know what size of array I need. All #defined values are in one file, so It's also fine. My problem was solved in comments to the post, but thank you for your answer.

mgueydan Over a year ago

I think there is a missunderstanding. You should use std::vector instead of array, even if you know the array size. Just because it's much safer. If you worry about performances, it's the very same thing. You may please refer to stackoverflow.com/questions/381621/…

Collectives™ on Stack Overflow

Segmentation fault - reading array of initialized pointers

1 Answer 1

2 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

2 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related