[select-Server] memory leaks ??

Jochen Kalmbach

"goto" ist immer ganz böse! und "break" in dem Fall auch
In beiden Fällen wird "tempBuf" vergessen...

R3dNeXX

ok, habt recht gehabt, stimmt natürlich, wenn Receive() schief läuft oder der Buffer leer ist, dann wird ja delete[] tempBuf nicht aufgerufen !

Habs jetzt so abgeändert:

if (!ClientSock.Receive(tempBuf, BUFFSIZE))
{
    delete[] tempBuf;
    break;
}
else if (strnlen(tempBuf, BUFFSIZE) == 0)
{
    delete[] tempBuf;
    goto START;
}

und so passt es jetzt auch !

Vielen Dank euch beiden

Martin Richter

der new auf den tempbuf ist doch unnötig. Warum legst Du den icht auf dem Stack an?
Und warum machst Du einen new für clientIP ?

R3dNeXX

Ich muss da mal blöd fragen: was spricht denn dagegen ??

Ich brauch einen LPTSTR für mein GetIPFrom und nen char-Pointer für Receive().
Sind beide vorgegebene, fertige Methoden in einer DLL.

Martin Richter

R3dNeXX schrieb:

Ich muss da mal blöd fragen: was spricht denn dagegen ??

Es ist langsamer. Fehlerbehafteter (wie Du siehst). Unnötig.

Solange ich solch kkleine Puffer nur temporär brauche ist der Stack, der richtige Ort und nicht der Heap!

R3dNeXX

Martin Richter schrieb:

R3dNeXX schrieb:

Ich muss da mal blöd fragen: was spricht denn dagegen ??

Es ist langsamer. Fehlerbehafteter (wie Du siehst). Unnötig.

Solange ich solch kkleine Puffer nur temporär brauche ist der Stack, der richtige Ort und nicht der Heap!

Ok, gut, dann musst du mir aber auch mal erkären, wie ich das jetzt löse, habe bis jetzt immer mit new meine Variablen aufm Heap angelegt, wenn ich wie hier z.B. nen char-Pointer brauch.

Habe jetzt ein Problem damit, das anders zu machen

WillieMacMoran

ganz normal *fg*

LPTSTR clientIP[255];

hast' halt den Vorteil, dass du dich nicht um die Zerstörung kümmern musst.

lg
Willie

R3dNeXX

ok, hast natürlich recht !

Ist eben so, wenn man sich an eine bestimmte Art gewöhnt hat, aber man lernt ja nie aus

Vielen Dank !

WillieMacMoran

Ich bin mir aber nicht sicher, ob du das [255] brauchst oder nicht - LPTSTR und dem ganzen Zeug geh ich auch eher aus dem Weg

Ja, ja, die Macht der Gewohnheit

lg
Willie

R3dNeXX

Ja, ja, die Macht der Gewohnheit

lg
Willie

Nuja, wenn ich das nicht mit dazuschreib, dann sagt er mir ja, dass clientIP nicht initialisiert ist und wenn ich dann damit was mache, dann knallts ja

Bin da grad auch noch etwas am Überlegen, kriege noch ne Access Reading Violation, wenn ich meinen Server schließe
die Stelle sieht so aus:

#ifdef _AFX_NO_OCC_SUPPORT
	if (m_hWnd != NULL)
		bResult = ::DestroyWindow(m_hWnd);
#else //_AFX_NO_OCC_SUPPORT
	if ((m_hWnd != NULL) || (m_pCtrlSite != NULL))
	{
		if (m_pCtrlSite == NULL)
			bResult = ::DestroyWindow(m_hWnd);
		else
			bResult = m_pCtrlSite->DestroyControl();    // hier bleibt er dann stehen
	}

Ich bin mir aber nicht sicher, ob du das [255] brauchst oder nicht - LPTSTR und dem ganzen Zeug geh ich auch eher aus dem Weg

und der Spaß dabei, ist, ich versuche das auch meistens, aber hier muss ich das so lösen, weil GetIPFrom und Receive Methoden aus ner DLL sind und die das so verlangen :p

ihoernchen

Das goto start würde ich durch ein continue ersetzen.
Macht in dem Fall das gleiche und schaut "besser" aus.

ihoernchen

statt
char *tempBuf = new char[BUFFSIZE];
char tempBuf[123];

LPTSTR ist, wenn ichs richtig sehe ein char* bzw. wchar_t*. das willst du nicht.
Du könntest TCHAR tempBuf[123] nehmen. Nur hast du UNICODE definiert ist TCHAR ein wchar_t -> also char nehmen (wie oben)

Martin Richter

WillieMacMoran schrieb:

Ich bin mir aber nicht sicher, ob du das [255] brauchst oder nicht - LPTSTR und dem ganzen Zeug geh ich auch eher aus dem Weg

Dann machst Du Dir aber mehr Arbeit.
TCHAR konsequent eingesetzt spart einen Haufen Arbeitund ist extrem portable innerhalb von Projekten die mit gesmischten Char-Sets auskommen müssen.

Martin Richter

WillieMacMoran schrieb:

ganz normal *fg*

LPTSTR clientIP[255];

hast' halt den Vorteil, dass du dich nicht um die Zerstörung kümmern musst.

Du meinst mit Sicherheit:

TCHAR clientIP[255];

Martin Richter

ihoernchen schrieb:

statt
char *tempBuf = new char[BUFFSIZE];
char tempBuf[123];

Man kann auch weiterhin Symbolische Wrete nehmen solange diese konstant sind.

char tempBuf[BUFFSIZE];

R3dNeXX

Ok, super, passt jetzt alles wie es soll !

Vielen Dank für die Hilfe und für die Aufklärung !!

Man will ja immer was lernen (was ich hier auch wieder habe)

R3dNeXX

Muss mich doch nochmal melden, hab trotzdem noch Probleme mit Leaks
und zwar sieht es jetzt so aus:

Detected memory leaks!
Dumping objects ->
f:\sp\vctools\vc7libs\ship\atlmfc\src\mfc\strcore.cpp(173) : {243} normal block at 0x018AB118, 57 bytes long.
Data: <, c$ ( > 2C 08 89 63 24 00 00 00 28 00 00 00 01 00 00 00
f:\sp\vctools\vc7libs\ship\atlmfc\src\mfc\strcore.cpp(173) : {241} normal block at 0x018AAEC8, 529 bytes long.
Data: <, c > 2C 08 89 63 00 02 00 00 00 02 00 00 01 00 00 00
Object dump complete.
The program '[4720] Server.exe: Native' has exited with code 0 (0x0).

Habe da dem Server 2 Nachrichten geschickt, der hat die angezeigt und dann hab ich ihn geschlossen und diese Leaks sind übrig geblieben.
Der aktuelle Code meines Client-Threads:

void CServerDlg::ThreadRunClient()
{
	const size_t BUFFSIZE = 512;
	CSingleLock sLck(&cs, FALSE);

	while (true)
	{
		if (ClientSock.IsConnected())
		{
			Sleep(500);
			CString message;
			char tempBuf[BUFFSIZE];
			int index = 0, index2 = 0;			// index -> für logList, index2 -> für strList

			if (!ClientSock.Receive(tempBuf, BUFFSIZE))
				break;
			else if (strnlen(tempBuf, BUFFSIZE) == 0)
				continue;
			else
			{
				for (int i = 0; i < BUFFSIZE; i++)
					message += tempBuf[i];
				CString data;
				TCHAR clientIP[255];

				ClientSock.GetIPFrom(clientIP, 255);
				data = "Daten erhalten [IP: "; data += clientIP; data += "]";
				sLck.Lock();
				logList.InsertString(index, data);
				logList.SetCurSel(index);
				strList.InsertString(index2, message);
				sLck.Unlock();
			}
		}
	}
	CloseMyHandle(hClient);
}

Da frage ich mich, was da nicht stimmt (falls da überhaupt was nicht stimmt ).
Falls es hilft, hier noch das OnClose():

void CServerDlg::OnClose()
{
	CloseMyHandle(hServer);
	ClientSock.Close();
	serversock.Close();
	CloseSocket();			// WSACleanup()
	EndDialog(0);
}

Vor allem wird im Output-Window ein Pfad angezeigt, den ich nicht vestehe, f:\... ist mein DVD-Laufwerk ! Oder hat das was andres zu bedeuten (Bitte nicht lachen, bin auch noch etwas müde ) ??

Vllt könnt ihr mir nochmal helfen !?

Martin Richter

Du hast irgendwo mit new einen CString allokiert und nicht freigegeben.
Du scheinstmir an sinnlosen Stellen immer mit new zu arbeiten und den entsprecheden delete zu vergessen.

R3dNeXX

Nein, hab ich nicht, hab grad nochmal durch den Code geschaut, habe nix mit new allokiert

Martin Richter

Siehst Du in Deinem Dump die Allkationsnummern 241+243. Sind die konstant, dann setze einfach mal

_CrtSetBreakAlloc(241);

An den Start Deines Programmes. Dann bekonmmst Du einen Debug Breakpoint und kannst im Callstack sehen, wer diesen new/malloc auslöst.

Siehe auch
http://msdn.microsoft.com/en-us/library/4wth1ha5(VS.80).aspx