static memberfunktion und static member

netrobot

die folgende codes habe ich irgendwo gelesen, es geht drum, die Fehler rauszufinden.

#include <string.h>
#include <stdio.h>

class Base
{
  private:
    char * name;
  public:
    Base(char * className)
    {
        name = new char[strlen(className)];
        strcpy(name, className);
    }

    ~Base()
    {
        delete name;
    }

    static void print(Base base)
    {
        printf("name: %s\n" , base.name);
    }
};

class Subclass : public Base
{
  public:
    Subclass(char * className) : Base(className)
    {
    }
};

int main()
{
    Base * pBase = new Subclass("test");
    Base::print(*pBase);
    return 0;
}

1. hier wird memory in main allokiert, aber woanders gelöscht.
2. durch static memberfunction auf nicht static member zugriffen(können wir das ausnutzen?)
3.Base-Teil wird gelöscht, aber nicht der Derived-Teil(memoryleak)
4. C, C++ gemischt
was ich nicht verstehe, wieso stürzt es bei

~Base()
    {
        delete name;
    }

ab?

volkard

static void print(Base base)
läßt sich ne kopie erzeugen ohne sinnvollen kopierkonstruktor.

netrobot

volkard schrieb:

static void print(Base base)
läßt sich ne kopie erzeugen ohne sinnvollen kopierkonstruktor.

ja, weil dies "base" ist ein vorläufiges object, beim Verlassen der Funktion, wird D'tor aufgerufen (Base-Teil ist damit gelöscht!)
Aber das hat mit dem Abstürzen nichts zu tun

David_pb

delete [] name;

sollts auch heissen.

virtual ~Base()

grüße

netrobot

stürzt trotzdem ab an der selben Stelle

David_pb

Weil der Speicher schon früher freigegeben wurde, würd ich ma sagen.

Ein Copy C'tor behebt das Problem:

Base( const Base &other )
{
	name = new char[ strlen( other.name )+1 ];
	strcpy( name, other.name );
}

Bei print wäre eine Übergabe als konstante Referenz aber sowiso besser:

static void print( const Base &base )
{
	printf( "name: %s\n", base.name );
}

Im ganzen also:

class Base
{
private:
	char *name;

public:
	Base( const char *className )
	{
		Init( className );
	}

	Base( const Base &other )
	{
		*this = other;
	}

	virtual ~Base()
	{
		delete [] name;
	}

	Base &operator=( const Base &other )
	{
		delete [] name;
		Init( other.name );
		return *this;
	}

	void Init( const char *className )
	{
		name = new char[ strlen( className )+1 ];
		strcpy( name, className );
	}

	static void print( const Base &base )
	{
		printf( "name: %s\n", base.name );
	}
};

class Subclass : public Base
{
public:
	Subclass( const char *className ) : Base( className )
	{}
};

grüße

netrobot

Weil der Speicher schon früher freigegeben wurde, würd ich ma sagen.

welche Speicher? this Pointer? es war nicht NULL,
name? es hatte auch einen Inhalt drin.
wenn es NULL war, kann ich trotzdem löschen, (etwa delete NULL)

David_pb

Der Speicher wird freigegeben und nicht auf NULL gesetzt und zwar genau an der Stelle an welcher die lokale Kopie von Base gelöscht wird, nämlich in der Funktion print.

static void print(Base base) // erstellt eine flache kopie (zeiger adresse von name wird einfach kopiert
{
     printf("name: %s\n" , base.name); // super, funktioniert
} // löscht die lokale kopie und ruft den Destruktor auf welcher den Speicher freigibt

Weil es sich um eine flache Kopie des Objekts handelt zeigen beide Zeiger (pBase->name und base.name) auf den gleichen Speicherbereich. Wenn beim verlassen des Programms nun pBase zerstört wird (sollte ohnehinn per delete geschehen) versuchst du einen Speicherbereich freizugeben der dir garnichmehr gehört.

Daher => Copy C'tor und Zuweißungs Operator für Base

grüße

evilissimo

welche Speicher? this Pointer? es war nicht NULL,
name? es hatte auch einen Inhalt drin.
wenn es NULL war, kann ich trotzdem löschen, (etwa delete NULL)

name natürlich, diese Variable wurde nicht kopiert, sondern es wurde nur eine flache Kopie davon gemacht. D.h. es wurde nur der Zeiger aber nicht die zeichenkette kopiert.

Und nach dem das Objekt base dann 2 mal existiert weil es kopiert wurde, aber beide male einen Zeiger name haben welche beide, auf Grund der flachen Kopie (weil kein Kopierkonstruktor vorhanden ist), auf den gleichen Speicherbereich zeigen, stürzt es beim zweiten aufruf von delete ab. Mal davon abgesehen das es delete[] heißen müsste. Aber das wurde ja bereits schon erwähnt.

Es gibt hier jetzt zwei Möglichkeiten:

Erste:

#include <string>
#include <iostream>

class Base
{
  private:
    std::string name;
  public:
    Base(std::string const & className)
    : name(className)
    {        
    }

    virtual ~Base()
    {
    }

    static void print(Base const & base)
    {
        std::cout << "name: " << base.name << std::endl;
    }
};

class Subclass : public Base
{
  public:
    Subclass(std::string const & className) : Base(className)
    {
    }
};

int main()
{
    Base * pBase = new Subclass("test");
    Base::print(*pBase);
    delete pBase;
    return 0;
}

Zweite:

#include <cstring>
#include <iostream>

class Base
{
  private:
    char * name;
  public:
    Base(char * className)
    : name(0)
    {       
        if(className){ 
            name = new char[std::strlen(className)+1]();
            std::strcpy(name,className);
        } 
    }

    Base(Base const & o)
    :name(0)
    {
        if(className){ 
            name = new char[std::strlen(className)+1]();
            std::strcpy(name,className);
        } 
    }

    virtual ~Base()
    {
        delete[] name;
    }

    Base & operator=(Base const & o)
    {
        Base tmp(o);
        if(tmp.name){ 
            name = new char[std::strlen(tmp.name)+1]();
            std::strcpy(name,tmp.name);
        }
        return *this;
    }

    static void print(Base const & base)
    {
        std::cout << "name: " << base.name << std::endl;
    }
};

class Subclass : public Base
{
  public:
    Subclass(std::string const & className) : Base(className)
    {
    }
};

int main()
{
    Base * pBase = new Subclass("test");
    Base::print(*pBase);
    delete pBase;
    return 0;
}

Ich würde natürlich die Erste Variante vorziehen, da Sie ersten weniger Fehleranfällig als die Zweite ist. Und zweitens viel weniger Schreibarbeit mit sich bringt.

BR
Vinzenz

David_pb

Naja, doppelt gemoppelt hält ja bekanntlich besser...

evilissimo

David_pb schrieb:

Naja, doppelt gemoppelt hält ja bekanntlich besser...

Dein Zuweisungsoperator ist nicht ganz ok deswegen hab ich das ganze noch mal gemacht Und während ich die Antwort geschrieben habe, hast du schon gepostet, deswegen lösch ich doch nicht meinen ganzen Post

Das Problem bei dir ist die Selbstzuweisung. Dabei wird der String gelöscht und kann aber nicht mehr kopiert werden.

BR
Vinzenz

camper

evilissimo schrieb:

David_pb schrieb:

Naja, doppelt gemoppelt hält ja bekanntlich besser...

Dein Zuweisungsoperator ist nicht ganz ok [...]

der copy-ctor ist noch schlimmer. man kann es nur immer wieder wiederholen: der aufruf des copy-operators im copy-ctor ist im grunde immer ein fehler.

evilissimo

camper schrieb:

evilissimo schrieb:

David_pb schrieb:

Naja, doppelt gemoppelt hält ja bekanntlich besser...

Dein Zuweisungsoperator ist nicht ganz ok [...]

der copy-ctor ist noch schlimmer. man kann es nur immer wieder wiederholen: der aufruf des copy-operators im copy-ctor ist im grunde immer ein fehler.

Da hab ich gar nicht drauf geachtet aber das ist wahr

BR
Vinzenz

David_pb

camper schrieb:

evilissimo schrieb:

David_pb schrieb:

Naja, doppelt gemoppelt hält ja bekanntlich besser...

Dein Zuweisungsoperator ist nicht ganz ok [...]

der copy-ctor ist noch schlimmer. man kann es nur immer wieder wiederholen: der aufruf des copy-operators im copy-ctor ist im grunde immer ein fehler.

Gibts auch noch Gründe?

evilissimo schrieb:

Das Problem bei dir ist die Selbstzuweisung. Dabei wird der String gelöscht und kann aber nicht mehr kopiert werden.

Das's richtig! Aber so gut dein Code auch is, es sind doch Fehler drinn!

Base(char * className) // sollte const char * sein
: name(0)
{      
	if(className){
		name = new char[std::strlen(className)+1]();
		std::strcpy(name,className);
	}
}

Base(Base const & o)
:name(0)
{
	if(className){ // className gibts an der Stelle nich
		name = new char[std::strlen(o.name)+1]();
		std::strcpy(name,o.name);
	}
}

Subclass(std::string const & className) : Base(className ) // std::string zu char * convertieren is ein zweckloses Unterfangen ;)
{
}

Das alles is schön und gut, warscheinlich hast du den Code nich getestet aber wenn du schon an meinem Zuweißungs Operator rummäckerst dann:

Base & operator=(Base const & o)
{
	Base tmp(o);
	if(tmp.name){
		name = new char[std::strlen(tmp.name)+1](); // Ein DICKES FETTES Speicherleak!!!
		std::strcpy(name,tmp.name);
	}
	return *this;
}

Sorry, musste sein!

grüße

evilissimo

yo das ist genau einer der Gründe welche ich gemeint habe und deswegen die Erste Variante vorziehe. (weil std::string so verwendet keine Mem-leaks verursacht und kein falsches löschen)

Nobody is perfect

BR
Vinzenz