speicherzugriffsfehler bei map destruktion

MFK

Treb schrieb:

naja wie gesagt, ich sehe in der map auch keinen sinn, mir gings dabei eigentlich nur ums finden in logarithmischer zeit ...

std::set?

Badestrand

Wird die map denn sonstwo noch modifiziert?

Treb

ja, ich finds eigentlich auch lustig, aber wie würdet ihr strings abspeichern,
und dann schnell einen ptr zu einen string suchen

wenn ich einen vector nehme, muss ich ja im schlimmsten fall alle element durch

Fellhuhn

Ah, du willst also zur Laufzeit die Strings nicht mit zB m_node_names[name] abrufen, sondern immer direkt mit m_node_names["narf"]? Was ja nur Sinn machen würde, wenn der String der mit "narf" referenziert wird in der Map irgendwo verändert wird. Und dort vermute ich mal deinen Fehler.

Treb

also ich werde jetzt wohl ein set nehmen,
die map wird nirgends modifiziert nur kopiert,

ich denke dabei geht dann irgendwas schief, ich finde übrigens dieses
autolöschen bei map irgendwie doof, sowas macht vector doch auch nicht ...

der eigentliche sinn war, später nodes zu erzeugen, welche einen ptr auf ihren namen haben, und da wollte ich dann einfach die oben gezeigte funktion nehmen,

im tree ist die map als member drin, und würde dann erst nach allen nodes destruiert,
somit wären alle ptr to string immer valid, und ich hätte pro node nur 4bytes verbraucht um den namem zu speichern

Fellhuhn

Jetzt interessiert mich der Sinn des ganzen doch irgendwie. Kann mir da nämlich dann, wenn nichts modifiziert wird, nichts drunter vorstellen.

Die Funktion insertNodeName(const std::string& name) gibt dann ja immer nur einen Pointer auf eine Kopie des Parameters name zurück. Warum dann nicht gleich "name" nehmen?

MFK

Treb schrieb:

ich denke dabei geht dann irgendwas schief, ich finde übrigens dieses
autolöschen bei map irgendwie doof, sowas macht vector doch auch nicht ...

Jeder Container der Standardbibliothek löscht seine Element im Destruktor.

Treb schrieb:

im tree ist die map als member drin, und würde dann erst nach allen nodes destruiert,

Kopierst du vielleicht den Tree selbst? Dann hast du hoffentlich auch einen passenden Copy-Konstruktor bzw. Zuweisungsoperator.

Treb schrieb:

somit wären alle ptr to string immer valid, und ich hätte pro node nur 4bytes verbraucht um den namem zu speichern

Dir ist aber schon klar, dass deine mit new angeforderten Strings auch Speicher verbrauchen?

Fellhuhn

MFK schrieb:

Jeder Container der Standardbibliothek löscht seine Element im Destruktor.

In dem Fall aber ja nur den Pointer.

MFK schrieb:

Dir ist aber schon klar, dass deine mit new angeforderten Strings auch Speicher verbrauchen?

Immer nur beim ersten Aufruf. Danach wird nur noch der Pointer auf jenen vorher erstellten String zurückgegeben.

EDIT:

Eine Methode die ich sauberer finde:

Statt einen Pointer auf einen Namen bekommt jeder Node eine TypeID (Enum). Dazu eine Memberfunktion die anhand der TypeID den Namen zurückgibt. Braucht noch weniger Speicher.

Was du willst ist ein std::set in deiner Tree-Klasse and ein std::set::const_iterator in deiner Node-Klasse.

Treb

ok, ich muss einen baum aufbauen, und der baum besteht aus vielen
sub-bäumen,

jeder node hat einen namen, von den namen gibts ca 50
die bäume werden aber viele 1000 nodes haben, wovon viele natürlich
den selben namen haben werden
(es wird eben ein such-graph, wo die nodes zustände sind, und man kann
verschiedene zustände auch öfter besuchen)

ich wollte mir nun das speichern des names im node selber sparen, und jeder
node sollte nur einen pointer auf den namen haben, weil es eben viel mehr nodes als namen geben wird ...

beim erzeugen der nodes brauchte ich nun einen ptr auf den namen, und so wollte ich die finden ...

hätte ich einen vector genommen

std::vector<std::string> m_node_names;

hätte ich immer über den ganzen vector iterieren müssen, um nachzusehen ob name schon drin, (hätte dann aber adresse sofort über &m_node_names[i] zurückgegen können)

die std::set version sieht nun so aus

const std::string * const HMM_Tree::insertNodeName(const std::string& name)
{
std::set<std::string>::iterator it = m_node_names.find(name);
if(it != m_node_names.end()) return &(*it);
// else
m_node_names.insert(name);
it = m_node_names.find(name);
return &(*it);
}

und irgendwie glaube ich, meine stl-implementierung hat da irgendwo ne macke,
dann nun bekomme ich

==14333== Conditional jump or move depends on uninitialised value(s)
==14333== at 0x8053733: std::_Rb_tree<std::string, std::string, std::_Identitystd::string, std::lessstd::string, std::allocatorstd::string >::_M_erase(std::_Rb_tree_nodestd::string (stl_tree.h:1321)
==14333== by 0x805375D: std::_Rb_tree<std::string, std::string, std::_Identitystd::string, std::lessstd::string, std::allocatorstd::string >::~_Rb_tree() (stl_tree.h:592)
==14333== by 0x80537B1: std::set<std::string, std::lessstd::string, std::allocatorstd::string >::~set() (stl_set.h:94)
==14333== by 0x80529CD: HMM_Tree::~HMM_Tree() (hmm_tree.cpp:55)

aber das nehme ich mal als "warnung" hin ...

MFK

Treb schrieb:

und irgendwie glaube ich, meine stl-implementierung hat da irgendwo ne macke

Und ich glaube, du zerlegst dir irgendwo in deinem restlichen Code den Speicher.

Du kannst deine Funktion übrigens etwas vereinfachen:

const std::string * const HMM_Tree::insertNodeName(const std::string& name)
{
    return &(*(m_node_names.insert(name).first));
}

Nein!
So macht man das nicht, du fügst beim einfügen einfach mit insert den Namen in das set ein, dann bekommst du ein std::pair zurück wovon der erste ein Iterator auf den String ist und diesen Iterator speicherst du als const_iterator-Attribut in deinem Node-Objekt.

Alles andere ist quark, besonders deine insertNodeName-Methode...

Treb

ok, ich entschuldige mich

HMM_Tree HMM_ToTree(const Hybrid_LR_HMM& hmm)
{
std::string name = hmm.getName();
std::size_t numStates = hmm.numStates();
Matrix<double> trans_probs(hmm.trans_probs());

HMM_Tree ret_val;

for(std::size_t i=0; i < numStates -1; i++)
  {
  std::stringstream ss;
  ss << name << i;
  std::string subNodeName;
  ss >> subNodeName;
  ret_val.insertNodeName(subNodeName);
  std::cout << "Inserting node " << subNodeName << std::endl;
  }
return ret_val;
}

ohne

return ret_val;

was ich wegen unsinnigkeit zuerst weggelassen habe, kommt der fehler von oben,
nun läufts sauber und ich kann den baum aufbauen,
danke für den tip mit dem set
und entschuldigung für die aufregung

lolz_ausgeloggt schrieb:

Nein!
So macht man das nicht, du fügst beim einfügen einfach mit insert den Namen in das set ein, dann bekommst du ein std::pair zurück wovon der erste ein Iterator auf den String ist und diesen Iterator speicherst du als const_iterator-Attribut in deinem Node-Objekt.

Alles andere ist quark, besonders deine insertNodeName-Methode...

ist schon abgeändert ...

Sehe ich das richtig, dass du in der Methode nur die Strings für den Baum einlesen willst?

Wie bzw. wo erzeugst du denn deine Knoten?

Diesen Schritt mit den Namen könntest du auch weglassen, wenn du Zugriff auf die Namen beim erzeugen der Knoten hast.