Delete eines dynamischen 2D Arrays verusacht Absturz

Achja und eine Zuweisungsfunktion wurde in den privaten Bereich von Map deklariert.

krümelkacker

Du machst es einem nicht leicht Dir zu helfen. Diese kurzen Code-Fragmente sind nicht besonders aufschlussreich. Schau nochmal in der FAQ nach Tipps bzgl Stellen von Fragen. Alternativ kannst Du Dir auch ein schlaues C++ Buch besorgen. Ich habe ein bisschen den Eindruck, dass Du das eine oder andere wichtige an C++ nicht verstanden hast.

Schonmal daran gedacht die STL zu verwenden?

std::vector<std::pair<int, int> >

Schon hat man eine einfachen leicht zu verwendende 2D-Vector.

HighLigerBiMBam

Falsch! Einen 2-D Vector hast du nur mit std::vector< std::vector < Tile > >

Ich bedanke mich für die Tips, aber ich möchte zuerst wissen, warum ein 2D Pointer nicht auf NULL zeigen kann, bevor ich was anderes probieren will.

Hier ein Beispiel, was das Problem betrifft.

class Map
{
    public :
        Map(){

            Mapx = 1;
            Mapy = 1;

            map_Data = Allocate2DArray < int > (Mapx, Mapy);

        }
        ~Map(){

            Free2DArray < int > (map_Data);

        }
        void MakeMap(unsigned int x, unsigned int y){

            FreeMap();

            Mapx = x;
            Mapy = y;

            Free2DArray < int > ( map_Data );
            map_Data = Allocate2DArray < int > (Mapx, Mapy);

        }
    private :
        void FreeMap(){

            Free2DArray < int > (map_Data);

            map_Data = NULL; // Fehler

            Mapx = 0;
            Mapy = 0;

        }
        int Mapx, Mapy;
        int** map_Data;
};

int main ()
{

    Map level;
    level.MakeMap(2,3);
    return 0;

}

Besitze ich ein falsches Verständniss für Pointer ?
Bitte klärt mich auf, was das Problem angeht.

Calk

1.: Warum löschst du dein 2D Array 2mal (einmal in Freemap und einmal nachdem du Mapx und Mapy x und y zugewiesen hast)?

Und ansonsten probier doch mal

*map_data=NULL;

Dann tust du ja nur einen normalen Zeiger auf NULL setzen und man könnte über diesen auch nicht irgendwodrauf zugreifen, da es ja indirekt ein NULL-Zeiger ist.

krümelkacker

Dman schrieb:

Ich bedanke mich für die Tips, aber ich möchte zuerst wissen, warum ein 2D Pointer nicht auf NULL zeigen kann, bevor ich was anderes probieren will.

Ich gehe davon aus, dass Du mit "2D Zeiger" einen Zeiger auf einen Zeiger meinst. Ein Zeiger kann nicht auf "Null zeigen". Aber ein Zeiger kann "Null sein". Das gilt natürlich auch für Zeiger auf Zeiger.

Dman schrieb:

Hier ein Beispiel, was das Problem betrifft.

class Map
{
    public :
        Map(){
            Mapx = 1;
            Mapy = 1;
            map_Data = Allocate2DArray < int > (Mapx, Mapy);
        }
        ~Map(){
            Free2DArray < int > (map_Data);
        }
        void MakeMap(unsigned int x, unsigned int y){
            FreeMap();
            Mapx = x;
            Mapy = y;
            Free2DArray < int > ( map_Data );
            map_Data = Allocate2DArray < int > (Mapx, Mapy);
        }
    private :
        void FreeMap(){
            Free2DArray < int > (map_Data);
            map_Data = NULL; // Fehler
            Mapx = 0;
            Mapy = 0;

        }
        int Mapx, Mapy;
        int** map_Data;
};

Du hast anscheinend die Dreierregel, obwohl ich 2mal darauf hingewiesen habe, nicht verstanden bzw nicht nachrecherchiert. Außerdem: Was für ein Fehler soll "Fehler" sein? Kannst Du Dir nicht denken, dass man als Helfer auch gern die Fehlermeldung lesen würde? So kann ich wieder nur RATEN. Und ich rate, dass Du keinen entsprechenden Header inkludiert hast, der das Makro NULL definiert. NULL ist nämlich kein Schlüsselwort. Verwende stattdessen einfach die Ganzzahlkonstante 0.

Als kleine Fingerübung ist dieser Ansatz schon OK, da Du dadurch Resourcenverwaltung und Co lernst. Aber Du kannst es auch VIEL einfacher haben, indem Du einfach einen std::vector als Datenelement verwendest.

inter2k3

Bezugnehmend auf deinen letzten geposteten code:

In MakeMap rufst du FreeMap auf. FreeMap ruft Free2DArray auf und setzt map_Data auf NULL. Soweit alles ok.

Rücksprung in Makemap. Du rufst Free2DArray wieder auf mit map_Data (jetzt NULL-Pointer). In Free2DArray dereferenziert du map_Data ---> schlechte Idee und dein Problem.

Nun gut, ich kann jetzt endlich einen Zeiger auf Zeiger auf NULL setzen.

Dass ich die Dreierregel so nicht nachrecherchiert stimmt nicht.
Soweit ich lesen konnte besagt diese, dass wenn ein Kontruktur, Destruktur oder eine Zuweisungsoperatorfunktion vom Programmierer angegeben ist man auch die anderen zwei deklarieren sollte.
Ich habe lediglich vergessen das im Beispiel anzugeben.

So jetzt werde ich mich darum bemühen, den Vector du benutzen.

Ich danke für eure Hilfe !

inter2k3

Die besagten drei sind:

copy constructor
assignment operator
destructor

Stell dir folgendes Szenario vor:
Du erstellst ein Objekt des Typs Map, welches als data-member Zeiger besitzt. Diese zeigen in deinem Fall auf im Konstruktor dynamisch allozierten Speicher.

Nun übergibst du das Map-Objekt per value an eine Funktion. Dabei wird es kopiert (copy constructor). Da du keinen angegeben hast, wird einer vom compiler erstellt, der allerdings nur eine flatcopy / shallow copy durchführt.
Will heissen, dass zwar der Pointer kopiert wird, aber die Kopie auf die gleiche Speicheradresse zeigt wie das Orginal.

Beim Verlassen der Funktion verliert das kopierte Objekt seine Gültigkeit und wird daher zerstört. Im von dir angegebenen Destructor gibst du den Speicher auf den der Pointer zeigt wieder frei.

Allerdings zeigt der Pointer des ursprünglichen Map-Objekts (von dem die Kopie angelegt wurde) weiterhin auf die Adresse --> ein dangling pointer.
Es wäre jetzt fatal die dort gespeicherten Daten zu benutzen, zu manipulieren oder zu versuchen, den Speicher freizugeben.

Die Lösung wäre also die Angabe eines copy constructors, der neuen Speicher alloziert, die Daten des Speicherbereis kopiert und den Pointer der Kopie entsprechend auf den neu allozierten Speicher setzt. Das nennt man deep-copy.

Ich hoffe es ist klar, dass in einem solchen Fall wohl auch unbedingt die Angabe eines assignment-operators nötig ist (tust du es nicht, werden memory leaks die Folge sein)
(dabei darauf achten, dass du auf self-assignment testest).

Einige Beispiele für den Fall, dass dir nicht bewusst ist, wann z.B. Kopien erstellt werden:

Initialisieren mit Objekt des gl. Typs
call-by-value bei Funktionsaufrufen
return-value
Verwendung der STL-Container oder der meisten STL-Algorithmen
etc.

ok, ich hoffe ich habe die Dreierregel jetzt verstanden.
Eine copy, bzw assigment Funktion für die Klasse Map wäre folglich :

Map::Map& operator=(const Map cpy)
{
   Mapx = cpy.Mapx;
   Mapy = cpy.Mapy;

   FreeMap();
   map_Data = Allocate2DArray < int > (Mapx, Mapy);

   // "Depp-copy"
   for (int i = 0;i < Mapy ; i++)
   {
      for (int j = 0;j < Mapx ; j++)
      map_Data[i][j] = cpy.map_Data[i][j];
   }

   return *this;
}

Wenn ich nun aber in jeder Session meines Programmes jeweils nur eine Instanz der Klasse Map enstehen lasse bin ich doch dazu befugt die Dreierregel zu brechen oder sehe ich das falsch.
Es gäbe ja keinen Grund sich vor falschen Pointern zu schützen , wenn man letztlich keine Kopie einer Map Isntanz vor hat/programmiert.

asc

Dman schrieb:

Wenn ich nun aber in jeder Session meines Programmes jeweils nur eine Instanz der Klasse Map enstehen lasse bin ich doch dazu befugt die Dreierregel zu brechen oder sehe ich das falsch.

Selbst dann solltest du sicher stellen, das keine unabsichtlichen Kopien/Zuweisungen im Code vorkommen (Deklariere sowohl den Kopierkonstruktor als auch Zuweisungsoperator privat, und implementiere sie nicht).

HighLigerBiMBam

Ergänzend zu asc kannst du auch eine Klasse uncopyable erstellen und davon erben. Oder du nimmt boost noncopyable.

krümelkacker

Dman schrieb:

ok, ich hoffe ich habe die Dreierregel jetzt verstanden.
Eine copy, bzw assigment Funktion für die Klasse Map wäre folglich :

Map::Map& operator=(const Map cpy)
{
   Mapx = cpy.Mapx;
   Mapy = cpy.Mapy;

   FreeMap();
   map_Data = Allocate2DArray < int > (Mapx, Mapy);
   
   // "Depp-copy"
   for (int i = 0;i < Mapy ; i++)
   {
      for (int j = 0;j < Mapx ; j++)
      map_Data[i][j] = cpy.map_Data[i][j];
   }

   return *this;
}

Wenn Du sagst "copy, bzw assigment Funktion" und dann nur den Zuweisungsoperator zeigst, hast Du dann den Kopierkonstruktor vergessen oder willst Du ihn einfach nicht zeigen? Falls ersteres: Da gehört noch ein Kopierkonstruktor dazu!

Ich hoffe, Du weißt, dass Kopierkonstruktor und Zuweisungsoperator ggf vom Compiler selbst -- eventuell falsch -- implementiert werden. Daher gibt's ja auch die Dreierregel.

So schreibt man auch keinen Zuweisungsoperator. Ich sehe zwar keinen Fehler, aber das sollte recht unperformant sein; denn wenn Du so etwas schreibst wie

a = b;

werden zwei Kopien erzeugt. Zunächst wird von b eine Kopie erzeugt (-> cpy) und dann kopierst Du den Inhalt von cpy noch nach a.

Ich verweise hier mal auf den Copy & Swap Trick.

Deine Klasse sollte (mit Copy-&-Swap) in etwa so aussehen:

class Map
{
public:
  Map();                    // Default-Konstruktor
  Map(int xdim, int ysim);
  Map(Map const&);          // Kopier-konstruktor
  ~Map();                   // Destruktor
  void swap(Map & that);    // Vertauscht den Inhalt zweier Objekte
  Map& operator=(Map temp) { swap(temp); return *this; }

  //... noch ein paar Operationen

private:
  int xdim_;
  int ydim_;
  ElemType** elems_;
};

inline void swap(Map & a, Map & b) {a.swap(b);}

wobei man bei swap natürlich nur die zwei ints und den Zeiger austauschen muss.

Wenn du es ohne Copy-&-Swap Trick machen willst, sollte es so aussehen:

class Map
{
public:
  Map();                    // Default-Konstruktor
  Map(int xdim, int ysim);
  Map(Map const&);          // Kopier-konstruktor
  ~Map();                   // Destruktor
  Map& operator=(Map const& temp);

  //... noch ein paar Operationen

private:
  int xdim_;
  int ydim_;
  ElemType** elems_;
};

...

Map& Map::operator=(Map const& temp)
{
  if (this==&temp) return *this; // Selbstzuweisung abfangen
  ...
  return *this;
}

Und wenn Du das fehlerfrei geschafft hast, dann kannst Du das Ding mit 'nem STL-Container komplett neu machen:

class Map
{
public:
  Map();
  Map(int xdim, int ysim);

private:
  int xdim_;
  int ydim_;
  std::vector<ElemType> elems_;
};

Beachte: Man muss hier gar keinen Kopierkonstruktor, Destruktor und Zuweisungsoperator selbst deklarieren, da die vom Compiler-generierten Versionen genau das richtige tun. Du solltest also beim Klassen-Design darauf Wert legen, dass benutzerdefinierter Destruktor und Kopier-Operationen erst gar nicht nötig sind, falls möglich.

Dman schrieb:

Wenn ich nun aber in jeder Session meines Programmes jeweils nur eine Instanz der Klasse Map enstehen lasse bin ich doch dazu befugt die Dreierregel zu brechen oder sehe ich das falsch.

Wenn so ein Objekt nicht kopieren brauchst, dann solltest Du es auch wenigstens verbieten, so dass ein Kopierversuch zu einem Kompilierfehler führt. Das kann man erreichen indem Kopierkonstruktor und Zuweisungsoperator privat deklariert werden und undefiniert bleiben -- oder alternativ die Klasse von so etwas wie boost::noncopyable erben lässt.

Mit welchem Buch lernst Du C++? Steht da so'was nicht drin? Hast Du Scott Meyers' "Effective C++" gelesen? Falls nicht, unbedingt kaufen!

kk