strcpy() Fehler der mich völlig verwirrt...
-
Hi Leute,
also ich hab ein Problem das ich absolut nicht verstehe.
Erstmal der Code meiner Klasse:
class Product { private: char* bezeichng; public: Product(char* bez = "default"); }; Product::Product(char* bez) { strcpy(bezeichng,bez); }Beim compilieren bekomme ich folgende Warnung:
warning C4996: 'strcpy' was declared deprecated
Wenn ich starte bekomme ich ein Laufzeitfehler, weil ein Maschinenbefehl von strcpy einen Fehler verursacht.
Was bitte ist an diesem Code falsch? Bin ich völlig blind?Vielen Dank schonmal im voraus...
-
Du bekommst einen Laufzeitfehler (und hast bei Sprachen
wie C/C++ da sogar Glück gehabt, daß du überhaupt auf den
Fehler hingewiesen wurdest), weil du den übergebenen
String ins Nirwana (oder sonstwohin) kopierst.Du mußt vorher Speicher allozieren:
bezeichnung = (char *) malloc(strlen(bez) * sizeof(char)); strcpy(bezeichng,bez);oder du benutzt gleich die in C++ definierte Klasse String, die
sich umsowas automatisch kümmert.Ich habe aber schon Jahre nicht mehr in C++ geproggt, also
keine Ahnung wie die anzuwenden ist.
-
Du has ja gar kein Speicher geholt.
-
sollte man nicht eher new verwenden da c++ ?
also eher sowas wie da unten...
zerstören über delete[]oder halt wie gesagt mit strings arbeiten...
bezeichnung = new Char[strlen(bez)]; strcpy(bezeichng,bez); delete[] bezeichnung;edit: nicht getestet, bin mir nich 100% sicher...
-
Hoi,
das...ConfusedGuy schrieb:
bezeichnung = new Char[strlen(bez)]; strcpy(bezeichng,bez); delete[] bezeichnung;solltest du nicht genau übernehmen; Das ist zwar richtig, jedoch sollte vorher geprüft werden, ob 'bez' ungleich NULL ist, sonst gibts ne Speicherverletzung!

Product::Product(char* bez) { if(bez != NULL) { bezeichng = new char[strlen(bez) + 1]; strcpy(bezeichng, bez); } }im Desturktor dann ein

Product::~Product(void) { delete [] bezeichng; }
-
Also das strlen(...)+1 kann man sich sparen.
Das Wort "Hallo" hat 5 Buchstaben. strlen liefert also 5. Somit liegt das H bei bez+0 und das o an der Speicherstelle bez+4 und somit völlig im bereich.speicherverletzung?? sicherlich. wenn bez nach der alloziierung ungleich 0 sein sollte, würde ich mir weniger sorgen um ne speicherverletzung als um extremen speichermangel überhaupt machen.
ansonsten ist der code natürlich lieb und fein und vorbildlich und es sollte ja niemanden nen finger abbrechen doch nen paar mehr zeilen zu tippen.
-
Alles ausser std::string ist inakzeptabel....
-
nobody schrieb:
Also das strlen(...)+1 kann man sich sparen.
Das Wort "Hallo" hat 5 Buchstaben. strlen liefert also 5. Somit liegt das H bei bez+0 und das o an der Speicherstelle bez+4 und somit völlig im bereich.jaja, das hab ich mir angewöhnt, ist hier zwar unnötig, aber verwende ich immer in nem anderen Kontext...schaden kanns nicht (es sei denn du bist arm und kannst dir keinen 'guten' PC leisten, dem nicht genug Arbeitsspeicher zur Verfügung steht)
nobody schrieb:
speicherverletzung?? sicherlich. wenn bez nach der alloziierung ungleich 0 sein sollte, würde ich mir weniger sorgen um ne speicherverletzung als um extremen speichermangel überhaupt machen.
du hats gar nix gecheckt oder ? ...also ich kann weit und breit keine Alloziierung von 'bez' finden...es geht nur um die Definition einer fehlerunanfälligen Methode...wenn also irgendein Spassvogel sowas macht:
char *ptr = NULL // Keine init! Product produktDemo(ptr);würde, nach deinem Programmierstil, das Programm in alle ewigen Software-Himmel katapultiert
...tja: GENAUER LESEN! -> LOLnobody schrieb:
ansonsten ist der code natürlich lieb und fein und vorbildlich und es sollte ja niemanden nen finger abbrechen doch nen paar mehr zeilen zu tippen.
tja, weißt du...du wärst sicherlich einer, bei dem ich mir nicht solche Arbeit machen würde...ist sowieso n bisschen erbärmlich ne [...], als ungereggter so eine Scheiße zu posten

............ schrieb:
Alles ausser std::string ist inakzeptabel....
Da hast du Recht, allerdings ist der erste Post auch 'im C-Stil' verfasst.
PS: nicht immer ist std::string sinnvoll - mal davon abgesehen, das ich immer noch nicht weiß, was er bzwecken will - bei kleinen OPs ist char schneller!

-
strlen(...)+1 ist richtig, da strcpy auch noch die Nullterminierung mit kopiert und dann ist Hallo 6 Zeichen lang.
Und wegen diesem ganzen Mist sollte man std::string nehmen. Das ist auch nicht sehr viel langsamer. Ich kannte mal ne Seite, da haben die Benchmarks gemacht, weiß die aber leider nicht mehr.mfg.
-
nobody schrieb:
Also das strlen(...)+1 kann man sich sparen.
Das Wort "Hallo" hat 5 Buchstaben. strlen liefert also 5. Somit liegt das H bei bez+0 und das o an der Speicherstelle bez+4 und somit völlig im bereich.Du hast das Nullbyte vergessen. Setzen, sechs.
@CodeFinder:
Und Du hast Dich von einem Troll aus der Fassung bringen lassen :p
-
WAH, ich bin wirklich blind. Ich habs verdient mal kräftig aufn Hintern zu bekommen

Jungs danke, und streitet nicht: eine Speicherverletzung wird es bei "bez" nie geben, siehe:Product(char* bez = "default");Vielen Dank das ihr einem Blinden geholfen habt...
PS: die Warnung kommt immernoch, aber jetzt geht es logischerweise; wegen der Warnung, keine Ahnung...
-
Tobias W schrieb:
Jungs danke, und streitet nicht: eine Speicherverletzung wird es bei "bez" nie geben, siehe:
Product(char* bez = "default");Und was hindert da einen bösen Programmierer daran, Product(0) aufzurufen?!
CodeFinder schrieb:
nicht immer ist std::string sinnvoll - mal davon abgesehen, das ich immer noch nicht weiß, was er bzwecken will
Hmm, möglicherweise soll er bezwecken, dass ein C++-Programmierer nicht mit Zeigern rumwurschteln muss, nur weil er eine Zeichenkette verarbeiten will.
CodeFinder schrieb:
bei kleinen OPs ist char schneller!
char ist natürlich schneller, (meistens) ein Byte zu verarbeiten ist ja auch nicht so schwer. Wenn du C-Strings meinst, dann schreib das auch. Und wieso meinst du das? Beweise?! Nach der String-Implementierung in der libstdc++ ist da kaum Overhead. Den einzigen Vorteil haben Char-Arrays, wenn man weiß, wie groß der String wird. Dann spart man sich den Heap-Krams.
-
.filmor schrieb:
Tobias W schrieb:
Jungs danke, und streitet nicht: eine Speicherverletzung wird es bei "bez" nie geben, siehe:
Product(char* bez = "default");Und was hindert da einen bösen Programmierer daran, Product(0) aufzurufen?!
vor allem ist die speicherverlatzung dann garantiert. denn CodeFinder hat es versäumt, diesen Fall richtig zu behandeln - folglich kommt es dann spätestens im destruktor zu undefiniertem verhalten.
im übriges sollte der konstruktor wohl besser ein const char* argument erhalten.
-
Hui...^^
________________________________________________________________________________________
@joomoo:joomoo schrieb:
strlen(...)+1 ist richtig, da strcpy auch noch die Nullterminierung mit kopiert und dann ist Hallo 6 Zeichen lang.
...mein ich doch, DANKE!

________________________________________________________________________________________
@LordJaxom:LordJaxom schrieb:
@CodeFinder:
Und Du hast Dich von einem Troll aus der Fassung bringen lassenJup da haste wohl Recht... *vor den kopf hau* ^^ ...danke!!!

BTW für unseren Troll (schöne Bezeichnung! ):
Das wär die Speicherbelegung: char szWelcome[] = "Hallo"; ------------------------- 0 | 1 | 2 | 3 | 4 | 5 | ---|---|---|---|---|----| H | a | l | l | o | \0 | <- "Du hast das Nullbyte vergessen." -------------------------________________________________________________________________________________________
@Tobias W:
Tobias W schrieb:
PS: die Warnung kommt immernoch, aber jetzt geht es logischerweise; wegen der Warnung, keine Ahnung...
hmm seltsam...vllt. mal vollständiger Rebuild...sicher das es anner gleichen Stelle auftritt ?
naja und zur Not (aber in diesem Fall wohl sehr dirty...es muss ja irg n Grund für die Warnung geben^^):// Achtung: Compiler abhängig! #pragma warning(disable: 4996) // Blockanfang in dem die Warnung auftritt: // ... // ... // ... // Blockende #pragma warning(default: 4996)________________________________________________________________________________________
.filmor schrieb:
CodeFinder schrieb:
nicht immer ist std::string sinnvoll - mal davon abgesehen, das ich immer noch nicht weiß, was er bzwecken will
Hmm, möglicherweise soll er bezwecken, dass ein C++-Programmierer nicht mit Zeigern rumwurschteln muss, nur weil er eine Zeichenkette verarbeiten will.Das war wohl n Missverständnis...
denn das: "das ich immer noch nicht weiß, was er bzwecken will" war nicht auf std::string, sondern auf die Frage dieses Threads bezogen
...wobei -LOL- mir gerade auffällt, dass das n ganz anderes Thema war...wohl verwechselt...tja daran war wohl dieser 'Troll' ((C) by LordJaxom - genial ) Schuld.Wenn du C-Strings meinst, dann schreib das auch.
sry natürlich waren C-Strings gemeint

Und wieso meinst du das? Beweise?! Nach der String-Implementierung in der libstdc++ ist da kaum Overhead. Den einzigen Vorteil haben Char-Arrays, wenn man weiß, wie groß der String wird. Dann spart man sich den Heap-Krams.
Hmm...Aber es liegt doch wohl auf der Hand das ein Char-Array (also ein C-String
) weniger Speicher verbraucht als ein Objekt der Klasse std::string! ...PS: "Den einzigen Vorteil haben Char-Arrays, wenn man weiß, wie groß der String wird."...jo klar!
________________________________________________________________________________________
denn CodeFinder hat es versäumt, diesen Fall richtig zu behandelnHuh!? ... Hab ich doch:
CodeFinder schrieb:
Product::Product(char* bez) { if(bez != NULL) { bezeichng = new char[strlen(bez) + 1]; strcpy(bezeichng, bez); } }oder nicht
... lass mich gern belehren
^^im übriges sollte der konstruktor wohl besser ein const char* argument erhalten
stimmt!
, gar nicht dran gedacht________________________________________________________________________________________
jo dat waren dann alle, Wow was n Post^^ ----> allen noch n schönen Abend

-
CodeFinder schrieb:
camper schrieb:
denn CodeFinder hat es versäumt, diesen Fall richtig zu behandeln
Huh!? ... Hab ich doch:
Product::Product(char* bez) { if(bez != NULL) { bezeichng = new char[strlen(bez) + 1]; strcpy(bezeichng, bez); } }und welchen wert hat bezeichng hier nach beendigung des konstruktors, wenn bez NULL war?
-
@camper:
Hmm, das weiß wohl nur der Herr im Himmel :p ... ok hast Recht
...also schließen wir das mit:Product::Product(const char* pszBezeichnung) { if(pszBezeichnung != NULL) { try { bezeichng = new char[strlen(pszBezeichnung) + 1]; strcpy(bezeichng, pszBezeichnung); } catch(...) { cout << "FEHLER" << endl; bezeichng = NULL; } } else bezeichng = NULL; // Hier folgt dann höchst warscheinlich weiterer Code... // ... }PS: Das ExceptionHandling ist ein bisschen primitiv aber...es kracht wenigstens nie :p (ein kleiner Bonus)
-
Einen Fehler hast du rausgehauen und einen neuen reingebaut. Du wirfst die Exception nicht weiter. Ich glaube kaum, dass du das bezweckts. Fals doch würde ich eher zu new(std::nothrow) raten. Das gibt NULL zurück wenn kein Speicher mehr frei ist und du sparst die hier verhältnismässig doch recht teuren Exceptions.
Mal ganz davon abgesehen brauchst du hier gar keinen try Block denn fals der Konstruktor durch eine Exception verlassen wird dann wird kein Destruktor aufgerufen der Probleme machen könnte.
Richtig wäre es also:
Product::Product(const char* pszBezeichnung) { if(pszBezeichnung != NULL) { bezeichng = new char[strlen(pszBezeichnung) + 1];I strcpy(bezeichng, pszBezeichnung); } else bezeichng = NULL; }wobei ich dann doch eher zu
Product::Product(const char* pszBezeichnung) { if(pszBezeichnung != NULL) { bezeichng = new char[strlen(pszBezeichnung) + 1]; strcpy(bezeichng, pszBezeichnung); } else throw my_exception(); }oder gar
Product::Product(const char* pszBezeichnung) { assert(pszBezeichnung != NULL); bezeichng = new char[strlen(pszBezeichnung) + 1]; strcpy(bezeichng, pszBezeichnung); }tendiren würde denn es würde mich wundern wenn bezeichng == NULL ein sinvoller Zustand ist und nicht ein Zombieobjekt.
PS: Das "u" in "bezeichng" würd ich mir auch noch leisten. Soviel Tiparbeit spart man da auch nicht.

-
Ben04 schrieb:
Einen Fehler hast du rausgehauen und einen neuen reingebaut. Du wirfst die Exception nicht weiter. Ich glaube kaum, dass du das bezweckts.
DOCH!!! -> Es ging mir darum ALLE möglichen Exceptions aufzufangen (-> catch(...))!!! und außerdem:
CodeFinder schrieb:
PS: Das ExceptionHandling ist ein bisschen!!!primitiv!!! aber...es kracht wenigstens nie [...]
Ben04 schrieb:
Mal ganz davon abgesehen brauchst du hier gar keinen try Block denn fals der Konstruktor durch eine Exception verlassen wird dann wird kein Destruktor aufgerufen der Probleme machen könnte.
...wo wird er denn 'durch eine Exception verlassen' ???
PS_1: Den Variablen Name "bezeichng" hab ich gar nicht erfunden, sondern der Herr Tobias W
außerdem fehlt da nicht nur ein 'u', sondern auch ein 'n' -LOL-PS_2: "falls" schreibt man mit doppel 'l'

PS_3:
tendiren würde denn es würde mich wundern wenn bezeichng == NULL ein sinvoller Zustand ist und nicht ein Zombieobjekt.
tja, bei mir is NULL der Beweis (das Zeichen) für einen ungültigen Zeiger, außerdem halt ich nicht viel von assert...tja, wohl Ansichtssache :p
PS_4: Musstest du jetzt meinen schönen Schlusspost so zerrütteln^^

-
CodeFinder schrieb:
Ben04 schrieb:
Einen Fehler hast du rausgehauen und einen neuen reingebaut. Du wirfst die Exception nicht weiter. Ich glaube kaum, dass du das bezweckts.
DOCH!!! -> Es ging mir darum ALLE möglichen Exceptions aufzufangen (-> catch(...))!!! und außerdem:
CodeFinder schrieb:
PS: Das ExceptionHandling ist ein bisschen!!!primitiv!!! aber...es kracht wenigstens nie [...]
da wird nichts sinnvoll gehandeled. nur gecatched. ist also nur Exception Catching - und das ist primtiv
(und widerspricht geradezu dem zweck von exceptions).
-
camper schrieb:
und das ist primtiv
=
CodeFinder schrieb:
ein bisschen primitiv
Ist das so schwer ?

...
camper schrieb:
da wird nichts sinnvoll gehandeled. nur gecatched
hmm komisch genau das wollte ich hier erreichen:
CodeFinder schrieb:
es kracht wenigstens nie
-
Tobias W schrieb:
PS: die Warnung kommt immernoch, aber jetzt geht es logischerweise; wegen der Warnung, keine Ahnung...
Naja, wenn man weiss, wie solche Funktionen zu handeln sind, kann man die Warnung getrost ignorieren. Ein
#define _CRT_SECURE_NO_DEPRECATE_vor_ den Includes sollte die Warnung zur Not auch abstellen.