Fun-Soft - Programmierforum

Autor

Thread - Seiten: > 1 <

000
27.03.2006, 10:01 Uhr
mike
Pinguinhüpfer
(Operator)

Hallo

Ich steht mom irgendwo voll auf der Leitung - ich bekomm ein double free und weiß nicht warum:
Phone.h

C++:

#ifndef phone_h___
#define phone_h___

const unsigned IS_IDLE    = 0x01;
const unsigned IS_PHONING = 0x02;
const unsigned IS_ACTIVE  = 0x03;
const unsigned NUMBER_IS_INVALID = 0x04;
const unsigned NUMBER_IS_BUSSY   = 0x05;
const unsigned NUMBER_IS_OK      = 0x06;

class Network; // Needed for setting the Network
class Phone
{
  static unsigned phone_counter_;

  protected:
    unsigned phone_status_;
    Network *network_;
    char *name_;

  public:
    Phone();
    Phone(unsigned phone_status, const char *name);
    ~Phone();

    unsigned GetStatus() { return phone_status_; }
    char *GetName() { return name_; }
    void SetStatus(unsigned phone_status);

    const Network &GetNetworkHandler();
    void SetNetwork(Network *&network);

    static unsigned GetPhoneCounter();
};

#endif

Phone.cpp

C++:

#include <cstring>
#include "phone.h"

unsigned Phone::phone_counter_ = 0;

Phone::Phone(unsigned phone_status, const char *name) : network_(0)
{
  phone_status_ = phone_status;
  phone_counter_++;

  name_ = new char[strlen(name) + 1];
  strcpy(name_, name);
}

Phone::Phone() : network_(0)
{
  phone_counter_++;
}

Phone::~Phone()
{
  delete[] name_;
}

void Phone::SetStatus(unsigned phone_status)
{
  phone_status_ = phone_status;
  phone_counter_--;
}

unsigned Phone::GetPhoneCounter()
{
  return(phone_counter_);
}

void Phone::SetNetwork(Network *&network)
{
  network_ = network;
}

Nun wird das ganze in main.cpp aufgreufen:

C++:

#include <iostream>
#include "network.h"
#include "phone.h"

using namespace std;

int main (int argc, char * const argv[]) {
  Network *theNetwork = new Network;
  // now the network is created

  //Creating phones
  Phone *p1 = new Phone(IS_IDLE | IS_ACTIVE, "test");
  p1->SetNetwork(theNetwork);

  // Register them to Network
  cout << "Main: " << &p1 << endl;
  theNetwork->RegisterPhone("+431234", p1);

  // Do somthing ...
  theNetwork->ListAllNumbers();

  //delete phones
  delete p1;

  // destroy the network
  delete theNetwork;

  return 0;
}

Network.cpp schaut noch so aus

C++:

void Network::RegisterPhone(const char *phone_number, Phone *&phone)
{
  NetWorkPhone *tmp = new NetWorkPhone;
  char *tmpchr = new char[strlen(phone_number) + 1];
  strcpy(tmpchr, phone_number);

  tmp->theNumber = tmpchr;
  tmp->thePhone = phone;

  phones_[index_counter_++] = tmp;
}

void Network::ListAllNumbers()
{
  NetWorkPhone *tmpnp = new NetWorkPhone;
  Phone *tmpph = new Phone;

  for(int i=0; i<index_counter_; i++) {
    tmpnp = phones_[i];
    *tmpph = *(tmpnp->thePhone);
    cout << "Number: " << tmpnp->theNumber << tmpph->GetName() << endl;
  }

  //delete tmpph; <---- da hauts ihn auf
  delete tmpnp;
}

Vorsichtshalber noch die Network.h - vl. hab ich da auch was falsch

C++:

#ifndef network_h___
#define network_h___

class Phone;

struct NetWorkPhone
{
  char *theNumber;
  Phone *thePhone;
};

class Network
{
  NetWorkPhone *phones_[64];
  unsigned index_counter_;

  public:
    Network();
    ~Network();

    void RegisterPhone(const char *phone_number, Phone *&phone);
    void ListAllNumbers();
};

#endif

Ich hab den tmpph kontrolliert - er wird nicht geändert (Adresse). Allerdings löscht das delete den pointer aus main.cpp p1

Ich hab mir sagen lassen - es ist ein newbie Fehler im Code - allerdings hilft mir das nicht weiter

Danke im Voraus & sry für den Code spam
lg
--

Dieser Post wurde am 27.03.2006 um 10:03 Uhr von mike editiert.

Profil || Private Message || Suche

Download || Zitatantwort || Editieren || Löschen || IP

001
27.03.2006, 11:31 Uhr
RHBaum

Gibt es nen Grund, warum du so extensiv mit dynamischn speicher arbeitest ?
Wenn eine Instanz den quasi selben livecycle hat wie sein "parent", und diese Instanz aufn Stack passt, warum dann dynamisch anlegen ?

Genau so wie Instanzen die nur fuer die funktion gelten ...

char *tmpchr = new char[strlen(phone_number) + 1];
warum arbeitest du nicht mit der STL ?

Hab grad mal versucht deine news gegen die deletes gegenzuchecken, aber ich komm immer durcheinander ^^
wenn unbedingt new brauchst, die zu nem definitiven zeitpunkt geloescht werden sollen, nimm mal std::auto_ptr<>

void Phone::SetNetwork(Network *&network)
das phone muss sein Network kennen,
das network halt aber auch ne liste seiner "Phones" ... Muss das so sein ? ^^ Da sind ja konflikte vorprogrammiert ^^

Ciao ...

Dieser Post wurde am 27.03.2006 um 11:34 Uhr von RHBaum editiert.

Profil || Private Message || Suche

Download || Zitatantwort || Editieren || Löschen || IP

002
27.03.2006, 11:49 Uhr
mike
Pinguinhüpfer
(Operator)

Hi!

Ich will bewusst keine STL o.ä. Hab jetzt gut 2 Jahre nicht programmiert und will mir das ganze Pointer/Refernz von Grund auf wieder aneignen

Das Phone muss sein Network kennen - allerdings muss das Network die Nummer kennen und das dazu gehörige Telefon

Ich hab mal GDB angeworfen.
Die Zeile
*tmpph = *(tmpnp->thePhone);
in ListAllNumbers scheint nicht zu stimmen - muss erst schaun was ich da falsch mach.

lg
--

Profil || Private Message || Suche

Download || Zitatantwort || Editieren || Löschen || IP

003
27.03.2006, 12:18 Uhr
RHBaum

Zitat:

will mir das ganze Pointer/Refernz von Grund auf wieder aneignen

Zu dem thema gehoert aber auch, sinvoll zu entscheiden, wo man pointer / referenzen nimmt ...

Profil || Private Message || Suche

Download || Zitatantwort || Editieren || Löschen || IP

004
27.03.2006, 12:41 Uhr
mike
Pinguinhüpfer
(Operator)

Naja - übers wo lässt sich streiten

is ja nur ein Übungsprogramm ...

Mein Problem ist
*tmpph = *(tmpnp->thePhone);
wenn ich tmpph löscht er auch das p1 in main - aber warum? Irgendwie steh ich voll auf der Leitung. Aber ich denke ich sollte ein overriding von = machen - nur geht das net mit dem default =?

thx!!!
lg
--

Profil || Private Message || Suche

Download || Zitatantwort || Editieren || Löschen || IP

005
27.03.2006, 13:21 Uhr
RHBaum

Zitat:

Aber ich denke ich sollte ein overriding von = machen - nur geht das net mit dem default =?

du meinst, weil du mit deinen new / deletes durcheinander gekommen bist willst du den = operator ueberladen ? ^^ Nee das meinst du jetzt nich ernst oder ?

ok, ueberlegen wir mal was dein
*tmpph = *(tmpnp->thePhone);
macht:

derferenziert tmpph und weisst dem das derefernzierte tmpnp->thePhone zu ...

Deine Klasse Phone hat keine eigene version des = operators, also nimmt es die builtin version: sprich alle members werden kopiert ....

protected:
unsigned phone_status_;
Network *network_;
char *name_;

Also alle zeiger werden einfach nur ausgetauscht.

also tmpph->network_ /tmpph->name_ und tmpnp->thePhone->network_ / tmpnp->thePhone->name_ zeigen nun auf den selben speicher ....

Zitat:

is ja nur ein Übungsprogramm ...

Ich find es quasi genau so wichtig sich guten c++ Stil anzueignen. Wenn man einmal den pceudo c stil intus (code sieht aus wie sternenhimmel bei nacht, also viele viele ****) kriegt man den kaum raus ....

Woran erkennt man guten C++ Stil ? Dein programm ist klar strukturiert und leserlich, deine eigenen Klassen lassen sich da wo es sinn macht wie normale builtin variablen verwenden ....

C++:

int main (int argc, char * const argv[]) {
  Network myNetwork:

  //Creating phones
  Phone p1(IS_IDLE | IS_ACTIVE, "test",myNetwork);

  // Register them to Network
  cout << "Main: " << p1 << endl;
  myNetwork.RegisterPhone("+431234", p1);

  // Do somthing ...
  PhoneList list = theNetwork.ListNumbers();

  return 0;
}

Wuerde ich beispielsweisse fuer besseren c++ stil halten ...
Ich mein ueben mit new und delete, kein Thema ... schmeiss den aber gleich wieder weg ^^

dein problem iss ja nich das du ned allokieren kannst, sondern in deinem programm das du den ueberblick verlierst ^^
Zum beispiel sollt man sich immer expliziet nen kopf machen, ob man variablen besser besser kopiert oder doch referenziert ... performance versus sicherheit ... und performance immer dann den vorrang geben, wo es wirklich notzwendig ist ...
Und saueber stil beim allokieren und deallokieren ist eben das man es dem leser deines Programmes so einfach wie moeglich macht, zu sehen, aha, da iss nen new, und da iss auch scho das delete zu. Viel mehr gibts da auch nich zu beachten (und natuerlich das man es nicht sinnlos einsetzt ^^).

Ciao ...

Dieser Post wurde am 27.03.2006 um 13:22 Uhr von RHBaum editiert.

Profil || Private Message || Suche

Download || Zitatantwort || Editieren || Löschen || IP

006
27.03.2006, 13:25 Uhr
mike
Pinguinhüpfer
(Operator)

Damn. Stimmt - jetzt hab ich mir selber in den Haxn geschossen

Is klar dass es net gehen kann ... Hab das gleiche Problem vor nem Monat in nem C Buch gelesen und jetzt Fall ich drauf rein >_<

thx!!!!
mfg
--

Profil || Private Message || Suche

Download || Zitatantwort || Editieren || Löschen || IP

007
27.03.2006, 13:31 Uhr
RHBaum

wie du siehst, ich sehs auch ned aufm ersten blick ! ^^

warum ? weil ich solche stolpersteine von haus aus umgehe ... man macht es nach ner weile automatisch nimmer.

Und wenn man mehr mit variablen aufn stack arbeitet, fallen die zuweisungen der pointer weg, sprich man fuehrt immer die operatoren (zuweisung) dann gleich am Object aus, man macht sich von haus aus gedanken ob man da implementieren muss oder nicht ...

Ciao ..

Profil || Private Message || Suche

Download || Zitatantwort || Editieren || Löschen || IP

Seiten: > 1 < [ C / C++ (ANSI-Standard) ]

ThWBoard 2.73 FloSoft-Edition
© by Paul Baecher & Felix Gonschorek (www.thwboard.de)

Anpassungen des Forums
© by Flo-Soft (www.flo-soft.de)

Sie sind Besucher: