user3296933
user3296933

Reputation: 39

Valgrind invalid read of size 4 errors?

Im getting this error in Valgrind after attempting to free a list.

This is the important parts of Valgrind

    ==12349== Invalid read of size 4
    ==12349==    at 0x8048BB4: DynArray::addMovie(Movie*) (in /home/admin/a2/mbd)
    ==12349==    by 0x8048D11: main (in /home/admin/a2/mbd)
    ==12349==  Address 0x4321060 is 0 bytes after a block of size 0 alloc'd
    ==12349==    at 0x402B454: operator new[](unsigned int) (in         /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
    ==12349==    by 0x8048B62: DynArray::DynArray() (in /home/admin/a2/mbd)
    ==12349==    by 0x8048C87: main (in /home/admin/a2/mbd)
    ==12349== 
    ==12349== Invalid read of size 4
    ==12349==    at 0x8048BB4: DynArray::addMovie(Movie*) (in /home/admin/a2/mbd)
    ==12349==    by 0x8048D97: main (in /home/admin/a2/mbd)
    ==12349==  Address 0x4321114 is 0 bytes after a block of size 4 alloc'd
    ==12349==    at 0x402B454: operator new[](unsigned int) (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
    ==12349==    by 0x8048B90: DynArray::addMovie(Movie*) (in /home/admin/a2/mbd)
    ==12349==    by 0x8048D11: main (in /home/admin/a2/mbd)
    ==12349== 
    ==12349== Invalid read of size 4
    ==12349==    at 0x8048BB4: DynArray::addMovie(Movie*) (in /home/admin/a2/mbd)
    ==12349==    by 0x8048E1D: main (in /home/admin/a2/mbd)
    ==12349==  Address 0x43211d0 is 0 bytes after a block of size 8 alloc'd
    ==12349==    at 0x402B454: operator new[](unsigned int) (in                         /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
    ==12349==    by 0x8048B90: DynArray::addMovie(Movie*) (in /home/admin/a2/mbd)
    ==12349==    by 0x8048D97: main (in /home/admin/a2/mbd)

Important parts of DynArray.cc

    DynArray::DynArray() {
      size = 0;
      array = new Movie *[size];
    }        

    void DynArray::addMovie(Movie* m) {
      size++;
      Movie **temp = new Movie *[size];
      for (int x = 0; x < size; x++) {
        temp[x] = array[x];
      }
      delete [] array;
      temp[size-1] = m;
      array = temp;
    }

    void DynArray::cleanup() {
      for (int x = 0; x < size; x++)
        delete array[x];
      delete [] array;
    }

    int main() {
      DynArray *a = new DynArray();
      Movie *m = new Movie("sa", 2129, C_COMEDY);
      a->addMovie(m);
      Movie *k = new Movie("sas", 4324, C_DRAMA);
      a->addMovie(k);
      Movie *l = new Movie("dsad", 43241, C_DRAMA);
      a->addMovie(l);
      for (int x = 0; x < a->size; x++) {
        a->array[x]->printMovie();
      }
      a->cleanup();
      delete a;
      return 0;
    }

The movie class is not really that specially I can post it if required

Upvotes: 2

Views: 2079

Answers (2)

gd1
gd1

Reputation: 11403

size is initially 0 and the allocated buffer is of 0 bytes. When the first movie is added, you increment size too early. It gets equal to 1 and you read array[0] in the for loop.

By the way, your resize strategy is deadly inefficient... consider doubling the size of the array as needed.

Upvotes: 1

Vlad from Moscow
Vlad from Moscow

Reputation: 311088

Function addMovie is invalid.

void DynArray::addMovie(Movie* m) {
  size++;
  Movie **temp = new Movie *[size];
  for (int x = 0; x < size; x++) {
    temp[x] = array[x];
  }
  delete [] array;
  temp[size-1] = m;
  array = temp;
}

You increased object size and tried to access an element of array with index size - 1 which does not exist in array. The loop has to be as

  for (int x = 0; x < size - 1; x++) {
    temp[x] = array[x];
  }

Upvotes: 2

Related Questions