C++, problem med vector av klass innehållande "const char*"

Permalänk
Medlem

C++, problem med vector av klass innehållande "const char*"

Hej gott folk!

Då satt man än en gång här med något man bara inte lyckas lösa. Jag sitter och bygger en applikation i Qt som bl.a nyttjar <windows.h>. I applikationen anropar jag "EnumWindowsProc(HWND hwnd, LPARAM lParam)". Information om de fönster jag hittar lagrar jag då i en vector innehållandes min klass "WindowsWindow". Bl.a. fönstertiteln lagrar jag för enkelhetens skull i en "const char*" då det blir smidigast i andra delar av applikationen.

Så till problemet. När jag sedan loopar igenom denna vector och skriver ut innehållet i t.ex. variabeln "const char* windowText" så upptäcker jag till min förskräckelse att alla fönster levererar fönstertiteln från det sista fönstret jag lägger till i vectorn. Fenomenet uppstår med alla variabler av "const char*" typ. T.ex. en "bool" däremot har rätt värde.

Det är inga problem att via mina get och set-metoder ändra variabeln och skriva ut det rätta.

OBS! I nedan har jag skalat bort väldigt mycket som inte är relevant så ni slipper se det. Är kanske inte ens relevant att nämna det men whatever..

Klassen:

class WindowsWindow { public: WindowsWindow(const char* windowText); static BOOL CALLBACK EnumWindowsProc(HWND hwnd, LPARAM lParam); const char* getWindowText(); void setWindowText(const char* windowText); static vector<WindowsWindow>* windows; private: const char* windowText; };

Så här ser min callback ut:

BOOL CALLBACK WindowsWindow::EnumWindowsProc(HWND hwnd, LPARAM lParam) { BOOL isVisible = IsWindowVisible(hwnd); if(isVisible) { char title[300]; GetWindowTextA(hwnd, title, sizeof(title)); cout << "HWND: " << hwnd << endl; cout << "Window title: " << title << endl; WindowsWindow window = WindowsWindow(title); windows->push_back(window); } return TRUE; }

Såhär loopar jag igenom:

for(vector<WindowsWindow>::iterator it = WindowsWindow::windows->begin(); it != WindowsWindow::windows->end(); ++it) { cout << it->getWindowText() << endl; }

Loopar jag däremot igenom och skriver ut något värde till, kanske en "int" så skriver getWindowText ut något skumt i stil med "ProgÃ)" där det istället skulle stått "Program manager":

for(vector<WindowsWindow>::iterator it = WindowsWindow::windows->begin(); it != WindowsWindow::windows->end(); ++it) { cout << it->getWindowText() << endl; cout << it->getPosX() << endl; }

Antar att det har något med pekare/referens att göra men jag är helt enkelt inte hajj nog på det för att lista ut vad jag gör för fel. Har självklart försökt googla men vet inte riktigt vad jag ska googla på för just detta.

Idéer?? Hoppas det är något löjligt enkelt som någon av er därute ser med en gång!

Stavfel
Visa signatur

Burk Nr.1 : Asus P8Z68 V-Pro | i7 2600k @ 4,4 | EVGA GTX780 Classified | Corsair Vengeance 8GB DDR3 1600Mhz | Corsair Obsidian 650D | Corsair TX650M | Corsair H100

Burk Nr.2 : MSI P35 Neo2 | Core 2 Duo E6850 @ 3,2Ghz | Powercolor Radeon HD5850 1GB | Corsiar Twin2X 6400 2x2GB | Corsair 550W

Permalänk
Medlem

Hej, kan du posta constructorn till WindowsWindow? Skulle tro att du bara kopierar pekaren du skickar in när du skapar WindowsWindow object, och det den pekar på går out of scope i slutet av if(isVisible) scopet.

Sedan är WindowsWindow window = WindowsWindow(title); fel sätt att skapa nya object på. Det ska vara WindowsWindow window(title);

for(vector<WindowsWindow>::iterator it = WindowsWindow::windows->begin(); it != WindowsWindow::windows->end(); ++it)

Detta kan du byta ut mot

for(WindowsWindow window : *WindowsWindow:windows)

Tror inte heller att det finns någon anledning till att vectorn windows är en pekare.

Permalänk
Medlem

BOOL CALLBACK WindowsWindow::EnumWindowsProc(HWND hwnd, LPARAM lParam) { BOOL isVisible = IsWindowVisible(hwnd); if(isVisible) { char title[300]; // Allokera 300 char på stacken. GetWindowTextA(hwnd, title, sizeof(title)); cout << "HWND: " << hwnd << endl; cout << "Window title: " << title << endl; WindowsWindow window = WindowsWindow(title); // Skapa en ny WindowsWindow som sparar en pekare till title. windows->push_back(window); // Lägg till en kopia av window i vectorn. } // Här slutar detta scope, så title tas bort från stacken. windowText i instansen du lagt till i vectorn pekar nu på något som inte längre finns => odefinierat beteende. return TRUE; }

Du måste alltså kopiera din sträng när du skapar en ny instans av WindowsWindow, t.ex. med strdup. Då måste du också ta bort strängen i WindowsWindow destruktor för att undvika minnesläckor, och då måste du även definiera kopieringskonstruktorn och kopieringsoperatorn för att undvika att flera instanser tror att de äger samma sträng. Implementera gärna detta för att lära dig hur man gör, och ta sedan bort det och använd std::string istället

Skrivet av scared:

Detta kan du byta ut mot

for(WindowsWindow window : *WindowsWindow:windows)

För att förtydliga så är denna syntax ny i C++11, så det krävs att du använder en kompilator som klarar det. Oftast så krävs det att man själv slår på något kompilatorflagga för att C++11/C++14 ska användas. Koden saknar dessutom ett : i WindowsWindow::windows, och det är mycket effektivare att använda referenser istället för att kopiera varje element:

for(auto &window: *WindowsWindow::windows) { // auto istället för WindowsWindow, kompilatorn vet redan vilken typ window har. std::cout << window.getWindowText() << std::endl; }

Om du använder C++11 så kan du även använda emplace_back istället för push_back, så blir det mindre och effektivare kod:

windows->emplace_back(title); // Notera title som argument, inte window.

emplace_back lägger till ett nytt objekt i vectorn genom att anropa klassens konstruktor med argumenten som ges till emplace_back (genom s.k. perfect forwarding). Detta gör att man inte behöver kopiera instansen som ska läggas till i vectorn, det nya elementet i vectorn skapas direkt på plats istället.

Permalänk
Medlem
Skrivet av perost:

För att förtydliga så är denna syntax ny i C++11, så det krävs att du använder en kompilator som klarar det. Oftast så krävs det att man själv slår på något kompilatorflagga för att C++11/C++14 ska användas. Koden saknar dessutom ett : i WindowsWindow::windows, och det är mycket effektivare att använda referenser istället för att kopiera varje element:

Alla moderna utvecklingsmiljöer för C++ har väl ändå minst C++11 aktiverat som standard?

Helt rätt dessutom med att man i det här fallet ska använda referenser i en s.k. range-based for-loop, skyller på att jag programmerat för mycket java det senaste.

Permalänk
Medlem
Skrivet av scared:

Alla moderna utvecklingsmiljöer för C++ har väl ändå minst C++11 aktiverat som standard?

Jag har ingen riktig koll på det, jag använder vim, clang och make när jag kodar C++ Men GCC kommer i alla fall inte börja använda C++11 som standardval förrän nästa år med GCC 6.0, Clang vet jag inte när de tänker byta. Så det är ju i alla fall bra att veta att C++11-stöd kan vara något som man behöver (och bör) slå på själv.

Permalänk
Medlem

Yes här kommer konstruktorn:

WindowsWindow::WindowsWindow(const char* windowText) { this->windowText = windowText; }

Då börjar jag förstå varför det går som det går. Men om title nu går out of scope hur kommer det sig att jag ändå får ut något i huvudtaget? Känns som det borde krascha istället.

Att jag skapar objektet fel får jag helt enkelt skylla på att jag är van vid Java och inte gjort min hemläxa! Däremot så undrar jag vad det orsakar för tokigheter att göra som jag gjort, det "fungerar" ju nu, men vad är det jag missar?

Jag ska ta åt mig det ni båda påpekat och styra upp saker och ting imorgon, är för trött nu. Men tack så mycket så länge, återkommer imorgon eller i helgen med resultat!

Stavfel
Visa signatur

Burk Nr.1 : Asus P8Z68 V-Pro | i7 2600k @ 4,4 | EVGA GTX780 Classified | Corsair Vengeance 8GB DDR3 1600Mhz | Corsair Obsidian 650D | Corsair TX650M | Corsair H100

Burk Nr.2 : MSI P35 Neo2 | Core 2 Duo E6850 @ 3,2Ghz | Powercolor Radeon HD5850 1GB | Corsiar Twin2X 6400 2x2GB | Corsair 550W

Permalänk
Medlem
Skrivet av miFFhoe:

Då börjar jag förstå varför det går som det går. Men om title nu går out of scope hur kommer det sig att jag ändå får ut något i huvudtaget? Känns som det borde krascha istället.

Det finns en anledning till att det kallas för odefinierat beteende, precis vad som helst kan hända. Har du tur så kraschar programmet vid såna här fel, men har du otur så får du fel resultat eller krascher i en helt annan del av programmet, eller något helt annat. För att citera Bjarne Stroustrup: "C makes it easy to shoot yourself in the foot; C++ makes it harder, but when you do it blows your whole leg off.".

När det gäller konstruktorn så bör du göra det till en vana att hellre använda initialiseringslistan istället. Se "Member initialization in constructors" på denna sida.

Permalänk
Skrivet av miFFhoe:

Att jag skapar objektet fel får jag helt enkelt skylla på att jag är van vid Java och inte gjort min hemläxa! Däremot så undrar jag vad det orsakar för tokigheter att göra som jag gjort, det "fungerar" ju nu, men vad är det jag missar?

Du har en pekare till ett godtyckligt ställe på call-stacke där title i EnumWindowsProc råkade ligga. När EnumWindowsProc returnerat ligger visserligen det data som var skrevs i title kvar, men när du gör ett par andra funktionsanrop kommer dessa data skrivas över av data som de nya funktionerna lägger på stacken. Att det, som du säger, "fungerar" beror bara på att datat på stack inte har blivit överskrivet än.

När du har liknande "konstiga" fel, kan du pröva att köra DrMemory och se om du får någon hjälp.

Permalänk
Datavetare

Enklaste lösningen på detta problem, om du nu vill köra med const char * i gränssnittet, är att byta ut typen på medlemsvariabeln windowText till std::string

d.v.s.

... const char* windowText; ...

ersätts med

... std::string windowText; ...

Din getWindowText() blir då istället

const char * getWindowText() const { return windowText.c_str(); }

std::string kommer göra en s.k. "deep-copy" när den (logiskt) skapas/tilldelas via en const char *.

Visa signatur

Care About Your Craft: Why spend your life developing software unless you care about doing it well? - The Pragmatic Programmer

Permalänk
Medlem
Skrivet av perost:

Det finns en anledning till att det kallas för odefinierat beteende, precis vad som helst kan hända. Har du tur så kraschar programmet vid såna här fel, men har du otur så får du fel resultat eller krascher i en helt annan del av programmet, eller något helt annat. För att citera Bjarne Stroustrup: "C makes it easy to shoot yourself in the foot; C++ makes it harder, but when you do it blows your whole leg off.".

När det gäller konstruktorn så bör du göra det till en vana att hellre använda initialiseringslistan istället. Se "Member initialization in constructors" på denna sida.

Där hade jag ju en hel del att ta till mig!

Skrivet av Ingetledigtnamn:

Du har en pekare till ett godtyckligt ställe på call-stacke där title i EnumWindowsProc råkade ligga. När EnumWindowsProc returnerat ligger visserligen det data som var skrevs i title kvar, men när du gör ett par andra funktionsanrop kommer dessa data skrivas över av data som de nya funktionerna lägger på stacken. Att det, som du säger, "fungerar" beror bara på att datat på stack inte har blivit överskrivet än.

När du har liknande "konstiga" fel, kan du pröva att köra DrMemory och se om du får någon hjälp.

Insåg efter att jag skrivit att så borde ha varit fallet haha. Men lätt för mig att att säga nu va?

Skrivet av Yoshman:

Enklaste lösningen på detta problem, om du nu vill köra med const char * i gränssnittet, är att byta ut typen på medlemsvariabeln windowText till std::string

d.v.s.

... const char* windowText; ...

ersätts med

... std::string windowText; ...

Din getWindowText() blir då istället

const char * getWindowText() const { return windowText.c_str(); }

std::string kommer göra en s.k. "deep-copy" när den (logiskt) skapas/tilldelas via en const char *.

Denna lösning var ju löjligt enkel, fungerade finfint.

Ett stort tack till alla ovan som bidragit!

Visa signatur

Burk Nr.1 : Asus P8Z68 V-Pro | i7 2600k @ 4,4 | EVGA GTX780 Classified | Corsair Vengeance 8GB DDR3 1600Mhz | Corsair Obsidian 650D | Corsair TX650M | Corsair H100

Burk Nr.2 : MSI P35 Neo2 | Core 2 Duo E6850 @ 3,2Ghz | Powercolor Radeon HD5850 1GB | Corsiar Twin2X 6400 2x2GB | Corsair 550W

Permalänk
Medlem
Skrivet av scared:

(...)

Tror inte heller att det finns någon anledning till att vectorn windows är en pekare.

Hur ska jag i så fall komma åt vectorn utanför callbackens scope? Ska jag skapa en vanlig vector, och sedan skapa en pekare som pekar mot vectorn och skicka med pekaren in i callbacken? Eller har jag missat ännu något vitalt?

meningsbyggnad
Visa signatur

Burk Nr.1 : Asus P8Z68 V-Pro | i7 2600k @ 4,4 | EVGA GTX780 Classified | Corsair Vengeance 8GB DDR3 1600Mhz | Corsair Obsidian 650D | Corsair TX650M | Corsair H100

Burk Nr.2 : MSI P35 Neo2 | Core 2 Duo E6850 @ 3,2Ghz | Powercolor Radeon HD5850 1GB | Corsiar Twin2X 6400 2x2GB | Corsair 550W

Permalänk
Medlem
Skrivet av miFFhoe:

Hur ska jag i så fall komma åt vectorn utanför callbackens scope? Ska jag skapa en vanlig vector, och sedan skapa en pekare som pekar mot vectorn och skicka med pekaren in i callbacken? Eller har jag missat ännu något vitalt?

Förstår inte riktigt problemet. Gör du det till en vanlig vector (inte en pekare till en vector då) så kan du ju komma åt den genom att bara göra WindowsWindow::windows (alternativt bara windows om du redan är i klassen). Så i callback funktionen blir det windows.push_back(window); istället för windows->push_back(window);

Permalänk
Medlem
Skrivet av scared:

Förstår inte riktigt problemet. Gör du det till en vanlig vector (inte en pekare till en vector då) så kan du ju komma åt den genom att bara göra WindowsWindow::windows (alternativt bara windows om du redan är i klassen). Så i callback funktionen blir det windows.push_back(window); istället för windows->push_back(window);

Och jag förstår inte hur jag tänkte! Tack haha.

Visa signatur

Burk Nr.1 : Asus P8Z68 V-Pro | i7 2600k @ 4,4 | EVGA GTX780 Classified | Corsair Vengeance 8GB DDR3 1600Mhz | Corsair Obsidian 650D | Corsair TX650M | Corsair H100

Burk Nr.2 : MSI P35 Neo2 | Core 2 Duo E6850 @ 3,2Ghz | Powercolor Radeon HD5850 1GB | Corsiar Twin2X 6400 2x2GB | Corsair 550W