Hab ich hier eventuell etwas nicht berücksichtigt?
-
Hallöchen,
Ich benötige eine Bestätigung, dass ich nicht den totalen Mist verbrochen habe. Es geht um folgendes:
Ich habe mehrere Funktionen bei denen ich einen Speicherbereich zurückgebe, die Größe dabei ist variabel. Etwa so:
Pseudocode: (syntaktisch falsch) char* Funktion() { char* buffer = new char* [20] buffer = "Bla bla bla" return buffer }Mir ist dabei klar, dass der Speicher nicht freigegeben wird und somit verfault. Das soll nicht sein, daher habe ich mir eine Methode überlegt, mit der ich der Funktion ein Objekt übergebe welches einen pointer enthält. Dann kann die Funktion den Pointer manipulieren wie sie will und Dinge hinein schreiben. Wenn das Objekt dann aus dem Scope fällt und der destruktor aufgerufen wird, wird der allokierte Speicher freigegeben. Das Objekt soll aber nicht innerhalb der Funktion erzeugt werden da der Speicher dann bei dem Rückgabewert der Funktion schon freigegeben werden würde. Das Objekt mit dem Pointer wird also im Hauptcode erstellt, von dem aus dann die Funktion aufgerufen wird.
Das sieht dann so aus:
Pseudocode: (syntaktisch falsch) char* Funktion( object_klasse &name) { name = new char* [20] name = "Bla bla bla" return name } Hauptcode: { object_klasse xyz //objekt mit pointer erstellen cout<<Funktion( xyz )<<endl } //hier wird der Speicher im Pointer mittels Destruktor des Objekts freigegebenIch hoffe es ist klar wie ich das meine. Der Pointer wird praktisch vor der Funktion außerhalb im Hauptcode erzeugt (in einem Objekt) und dann übergeben an die Funktion. Die Funktion schreibt ihre Werte rein und gibts zurück. (Dabei wird das Objekt direkt als Referenz benutzt und KEINE Kopie des Objekts).
Als richtiger Code nun sieht das so aus:
#include <iostream> using namespace std; class pointer_class { private: int size; public: char* pointer; bool change_size(int new_size, bool flag = false); //Used to reallocate new memory //true = save old content //false = clear all bool write( char*, int = -1); pointer_class( int ); ~pointer_class( void ); }; bool pointer_class::write( char* new_content, int length) { if (length == -1) { length = strlen( new_content ); } if (length <= size) //only if pointer is holding enough memory { memcpy(pointer, new_content, length); return true; } return false; } bool pointer_class::change_size(int new_size, bool flag) { //flag true = keep content //flag false = clear content if (flag == true) //keep content { if (size > 0) { char* old_content = new char [size]; //allocate memory for old content to keep it memcpy(old_content, pointer, size); //save old content delete [] pointer; //delete pointer for reallocating soon with new size pointer = new char [new_size]; //allocate new memory memset(pointer, 0, new_size); //set all to zero memcpy(pointer, old_content, new_size); //fill in old memory till end of new buffer or till everything is in delete [] old_content; //delete temporary buffer for old content; size = new_size; //set new size return true; } pointer = new char [new_size]; memset(pointer, 0, new_size); size = new_size; return true; } else //clear content { if (size > 0) { delete [] pointer; } pointer = new char [new_size]; memset(pointer, 0, new_size); size = new_size; return true; } return false; } pointer_class::pointer_class( int p_size) { if (p_size != -1) { pointer = new char [size]; memset(pointer, 0, size); size = p_size; } else { pointer = NULL; size = 0; } } pointer_class::~pointer_class( void ) { if (size > 0) { delete [] pointer; } } char* testfunction( pointer_class &ext_buffer) { ext_buffer.change_size(4096); //change size of buffer and zero everything memcpy(ext_buffer.pointer, "DIES IST DER NEUE INHALT DES SPEICHERBEREICHS", strlen("DIES IST DER NEUE INHALT DES SPEICHERBEREICHS")); //set zero for testing return ext_buffer.pointer; } int main() { pointer_class buffer(-1); cout<<testfunction(buffer)<<endl; cin.get(); return 0; }Mir ist bewusst, ich begehe da schwere OOP-Fehler da der Pointer im Public bereich ist und somit frei manipuliert werden kann. Doch anders lässt es sich manchmal nicht machen (da manche WinAPI-Funktionen den reichen Pointer als Parameter benötigen und wenn der geschützt ist die Funktion einen Zugriffsfehler rausgibt). Daher brauche ich manchmal den ungeschützten reinen pointer aus dem Objekt welcher deshalb public ist.
Das Ganze wird eine recht komplexe Änderung in meinem Programm werden daher würde ich gern vorher wissen, ob der Code so korrekt ist. Es gibt bei mir keine Fehlermeldungen.
Wichtig ist, dass keinerlei Dinge im Speicher vergessen werden und keine Zugriffsfehler oder Ähnliches auftreten.
Wär' also nett wenn sich ein Kundiger mal den Code ansehen könnte und schaut ob alles ok soweit ist. Wie gesagt, das wären recht große Einschnitte wenn ich den Code einbaue, dann sollte er auch gehen. Nicht, dass später bei längerer Benutzung Speicherlöcher auftreten die dann schwere Fehler verursachen.mfg
Ynnus
-
mal davon abgesehen, daß dein code so grottenschlecht ist, daß sich mir die zehnägel kräuseln, scheint er korrekt zu sein
-
Lern C++. Im Moment programmierst du C/C++.
-
volkrad hat recht. Außerdem würde ich den pointer wohl durch ein to_raw_pointer() methode ersetzen.
Btw: Das nennst sich smart pointer, kannst ja mal danach googlen.
-
das ist schon realesiert und heißt std::vector
const char * testFunction(std::vector<char> &buffer) { const char *str = "DIES IST DER NEUE INHALT DES SPEICHERBEREICHS"; size_t len = strlen(str); buffer.assign(str, str + len + 1);//including the terminal NULL return &buff[0]; } int main() { std::vector<char> buffer; cout << testfunction(buffer)->c_str() << endl; cin.get(); return 0; }das geht aber einfacher mit std::string
const std::string testFunction() { std::string result = "DIES IST DER NEUE INHALT DES SPEICHERBEREICHS"; return result; } int main() { cout << testFunction().c_str() << endl; cin.get(); return 0; }
-
ness schrieb:
Btw: Das nennst sich smart pointer, kannst ja mal danach googlen.
nennt sich std::vector dann auch "smart pointer" ?
-
const char * testFunction(std::vector<char> &buffer) { const char *str = "DIES IST DER NEUE INHALT DES SPEICHERBEREICHS"; size_t len = strlen(str); buffer.assign(str, str + len + 1);//including the terminal NULL return &buff[0]; } int main() { std::vector<char> buffer; cout << testfunction(buffer)->c_str() << endl; cin.get(); return 0; }Wenn ich das so sehen sieht das so ähnlich aus wie die Methode die ich verwende. Nur, dass ich eben meine eigene Klasse erstelle die den Speicher managed. Abgesehen davon, dass euch mein C/C++ Stil nicht gefällt, wenn's sich ohne Speicherlöcher so machen lässt, wär' mir das schon recht.
Die C++ Umsetzungen mit Vector und String-Klasse könnte ich mir ja auch mal zu Gemüt führen aber im Moment gefällt es mir so (umständlich) noch ganz gut.
-
Ynnus schrieb:
Die C++ Umsetzungen mit Vector und String-Klasse könnte ich mir ja auch mal zu Gemüt führen aber im Moment gefällt es mir so (umständlich) noch ganz gut.
Musst du das Rad unbedingt neu erfinden ?
-
Offensichtlich sollte das eine klasse sein in der du c-strings oder auch irgendwelche anderen daten speichern willst. Wenn das so ist dann solltest du die länge der gültigen daten ( falls kein c-string ) speichern.
bool pointer_class::write( char* new_content, int length) { if (length == -1) { length = strlen( new_content ); // du hast das \0 am ende vergessen // warum kopierst du hier nicht ?? } if (length <= size) //only if pointer is holding enough memory { memcpy(pointer, new_content, length); return true; } // wenn length >= size warum rufst du nicht gleich change_size auf ?? return false; }Ich würde pointer trotzdem nicht public machen und eine getter function schreiben
zb.char * get_pointer() { return pointer; }Denn sonst könnte man irrtümlicherweise den pointer von ausserhalb der klasse überschreiben.
deine testfunction() müsste dann so aussehen:char* testfunction( pointer_class &ext_buffer){ ext_buffer.change_size(4096); //change size of buffer and zero everything // memcpy(ext_buffer.pointer, "DIES IST DER NEUE INHALT DES SPEICHERBEREICHS", strlen("DIES IST DER NEUE INHALT DES SPEICHERBEREICHS")); //set zero for testing // wozu hast du die write function ?? ext_buffer.write( "DIES IST DER NEUE INHALT DES SPEICHERBEREICHS", strlen("DIES IST DER NEUE INHALT DES SPEICHERBEREICHS")); return ext_buffer.get_pointer(); }Kurt
-
Ynnus schrieb:
Pseudocode: (syntaktisch falsch) // schon mal ganz schlecht char* Funktion() { char* buffer = new char* [20] // das verwirrt nur, deshalb immer richtig schreiben buffer = "Bla bla bla" // std::strncpy nehmen return buffer }Mir ist dabei klar, dass der Speicher nicht freigegeben wird und somit verfault. Das soll nicht sein, daher habe ich mir eine Methode überlegt, mit der ich der Funktion ein Objekt übergebe welches einen pointer enthält. Dann kann die Funktion den Pointer manipulieren wie sie will und Dinge hinein schreiben. Wenn das Objekt dann aus dem Scope fällt und der destruktor aufgerufen wird, wird der allokierte Speicher freigegeben.
Richtig. Gute Idee.
Das Objekt soll aber nicht innerhalb der Funktion erzeugt werden da der Speicher dann bei dem Rückgabewert der Funktion schon freigegeben werden würde. Das Objekt mit dem Pointer wird also im Hauptcode erstellt, von dem aus dann die Funktion aufgerufen wird.
Du musst einen Copyconstructor und einen operator= definieren.
#include <iostream> using namespace std; class pointer_class // Klassennamen werden meist groß geschrieben { private: int size; // sollte size_t sein, weil z.b. strlen auch mit size_t arbeiten char* pointer; // siehe data() public: const char* getData() const { return pointer; } // edit: es sollte getData und nicht nur "data" heißen bool change_size(int new_size, bool flag = false); // die funktionen von variablen sollten durch den namen erkennbar sein // vielleicht solltest du dich entscheiden, ob du minimale prototypen schreibst, oder immer gleich die namen mit angibst bool write( char*, int = -1); pointer_class( int ); ~pointer_class( void ); // void muss nur in c da stehen }; bool pointer_class::write( char* new_content, int length) { if (length == -1) { length = strlen( new_content ); } if (length <= size) //only if pointer is holding enough memory { std::strcpy(pointer, new_content); // strcpy, weil das ziel groß genug ist return true; } return false; } bool pointer_class::change_size(int new_size, bool flag) { //flag true = keep content //flag false = clear content if (flag) { if (size > 0) { /* char* old_content = new char [size]; //allocate memory for old content to keep it memcpy(old_content, pointer, size); // hier wieder dasselbe mit std::strcpy delete [] pointer; //delete pointer for reallocating soon with new size pointer = new char [new_size]; //allocate new memory memset(pointer, 0, new_size); //unnötig, wird sowieso gleich überschrieben memcpy(pointer, old_content, new_size); //fill in old memory till end of new buffer or till everything is in delete [] old_content; //delete temporary buffer for old content; size = new_size; //set new size return true; */ char* content = new char[new_size]; std::strncpy(content, pointer, new_size); delete [] pointer; pointer = content; } else { pointer = new char [new_size]; memset(pointer, 0, new_size); } size = new_size; return true; } else //clear content { if (size > 0) delete [] pointer; pointer = new char [new_size]; memset(pointer, 0, new_size); size = new_size; return true; } return false; } // lieber zwei Konstruktoren nehmen, einer mit size_t und einer ohne argumente pointer_class::pointer_class( int p_size) : size(p_size) // ist in mehr als 50% der fälle richtig { if (p_size > -1) // wir wollen schließlich keine ungültigen argumente für new { pointer = new char [size]; memset(pointer, 0, size); } else { pointer = 0; // NULL ist deprecated (oder so) size = 0; } } pointer_class::~pointer_class( void ) { if (size) delete [] pointer; }Mir ist bewusst, ich begehe da schwere OOP-Fehler da der Pointer im Public bereich ist und somit frei manipuliert werden kann. Doch anders lässt es sich manchmal nicht machen (da manche WinAPI-Funktionen den reichen Pointer als Parameter benötigen und wenn der geschützt ist die Funktion einen Zugriffsfehler rausgibt). Daher brauche ich manchmal den ungeschützten reinen pointer aus dem Objekt welcher deshalb public ist.
Schau dir mal std::string::data() und std::string::c_str() an.
Ansonsten ist grundsätzlich von ungeschütztem Datenverkehr abzuraten.
Das sah eigentlich alles richtig aus, habe es aber nicht compiliert.
Sieh das aber nur als Übung an und benutze ansonsten immer std::string.mfg
-
Wenn du weiter so programmierst wird dein Programm aber nicht gut!
-
ness schrieb:
Btw: Das nennst sich smart pointer, kannst ja mal danach googlen.
Da wird zwar auch Speicher freigegeben, aber das hier ist immer noch ein Container und ein std::string/vector<char> Ersatz.
mfg
-
terraner schrieb:
Du musst einen Copyconstructor und einen operator= definieren.
oder deine Klasse von boost::noncopyable erben lassen
-
@Ynnus
ich würde dir auch raten, dich mit den stl-klassen vector und string zu befassen. das ist zwar erst mal ein gewisser lernaufwand, der zahlt sich aber in jedem fall aus!
-
Wenn du weiter so programmierst wird dein Programm aber nicht gut!
Ok, ich hab ja schon verstanden, dass ich das nur unnötig kompliziert mache und besser auf vorhandene C++ Klassen zurückgreifen. Aber mal ganz unabhängig vom Gefallen am Programmierstil, solange keine Fehler drinne sind, hat es doch keine Auswirkungen darauf, wie gut das Programm wird.
Vielleicht darauf, dass es ein bisschen mehr Rechenleistung benötigt oder weniger Schleifendurchläufe pro Sekunde hinlegt. Aber sonst hat doch der Stil des Programmes wenig Auswirkungen auf das Resultat.
Wo genau ist denn der Knackpunkt (außer, dass ich C unc C++ mische) der das Programm so schlecht macht? Irgendwelche groben Verstöße beim Speichermanagement? Und das Variablen mal public sind lässt sich manchmal leider (scheinbar?) nicht umgehen. Beispiel: Die WinAPI benötigt des öffteren einen Speicherbereich um dort Werte hinein zu schreiben. Wenn ich nun den Pointer im Private Bereich des Objekts habe, wie kann ich den dann übergeben? Dann wird mir das Programm wohl mit einem Schreibsfehler abschmieren weil jemand in den Private-Bereich eines Objekts schreiben wollte. Oder gibt es da mir unbekannte Möglichkeiten, doch in die Private Variablen zu schreiben von Außerhalb? Ist ja auch nicht Sinn von OOP, daher hab ich die Variable gleich public gesetzt.
-
was spricht gegen eine get-memberfunktion?
-
public var schrieb:
was spricht gegen eine get-memberfunktion?
Ist der Private-Breich eines Objekts nicht in so fern geschützt, dass man Veränderungen NUR innerhalb der Klasse und deren Methoden vornehmen kann? Selbst wenn ich dann den Pointer per return zurückgebe und an die WinAPI-Funktion übergebe, sobald die Funktion den Inhalt ändern will, stürzt das Programm ab. Oder liege ich da falsch?
-
Liegst falsch!
-
Ich bin ja froh, dass einige hier so humorvoll sind. Hast du keinen Nick oder wie?
Ich hab's jetzt umgestellt, der Pointer ist nun private und wird per get_pointer() zurückgegeben. Ich hatte wohl damals was unglücklich falsch gemacht wodurch ich nun irrtümlich dachte, der Speicherbereich sei besonders geschützt, sodass man da eben nur durch klasseneigene Methoden reinschreiben kann. Damals gabs eben, wie gesagt, einen Speicherfehler, damit hatte sich das dann für mich erledigt.
Aber ich habs jetzt umgeändert.
-
Würde nicht auto_ptr aus der STL (memory) das selbe machen?
-
gurru schrieb:
Würde nicht auto_ptr aus der STL (memory) das selbe machen?
hier is es etwas doof, da der pointer keinen op[] hat.(Ansonsten is es natürlich ok, wenn man mit dem vergleicht, was der threadstarter benutzt
)