[gelöst]Schleife geht nicht, handweg geht (C4722)

stefanjann

Hallo,
der Titel sagt wenig aus, aber irgendwie tu ich mich schwer einen Titel zu finden.
Also Schritt für Schritt.
Ich bau gerade eine Midi-Klasse. (Also Musik-Keyboard am Computer)
Bei Midi habe ich folgt gekapselt:
Midi
=> enthält X Ports (Eingang)
=> enthält X Ports (Ausgang)

Jeder Port enthält 16 Channel
=> Eingangsport also 16 EingangsChannel
=> Ausgangsport also 16 AusgangsChannel

Damit das ganze weniger Arbeit ist und auch in DLLs passt habe ich Interfaces.

class PortInterface
class Port : virtual public PortInterface

class PortIn : virtual public Port
class PortOut : virtual public Port

Natürlich werden noch mehrere Klassen geerbrt, sonst würde das mit den Interfaces wenig Sinn machen.

So das war der Aufbau (grob und in Auszügen) und jetzt zum Problem:

CMidiPort::~CMidiPort()
{
	ChannelCloseAll();
}

typedef size_t DHandleMidiPort;

void CMidiPort::ChannelCloseAll()
{
	/* Version 1 (geht nicht)
	for (DHandleMidiPort Pos(ChannelCount()-1); Pos>=0; --Pos)
	{
		ChannelClose(Pos);
	}
	Version 2 (geht, aber schlecht): */
	ChannelClose((DHandleMidiPort)0); // Wegen Überladung mit Zeigern schnell casten
	ChannelClose(1);
	ChannelClose(2);
	ChannelClose(3);
	ChannelClose(4);
	ChannelClose(5);
	ChannelClose(6);
	ChannelClose(7);
	ChannelClose(8);
	ChannelClose(9);
	ChannelClose(10);
	ChannelClose(11);
	ChannelClose(12);
	ChannelClose(13);
	ChannelClose(14);
	ChannelClose(15);
}

Die Funktion ChannelCount() gibt (derzeit) immer 16 zurück. (Soll sich später ändern)

Wenn ich jetzt meinen Compiler anwerfe, dann klappt alles. Da ich aber später auch mal mehr als 16 Kanäle haben werde (Software-Mapper) dann würde ich gerne mit Schleife arbeiten. Also habe ich Version 1 eingetragen und promt bekomme ich den folgenden Linker-Fehler:

1>u:\sdks\MySDK\src\cmidiport.cpp(34) : warning C4722: "CMidiPort::~CMidiPort": Der Destruktor gibt nie Werte zurück, möglicherweise ist ein Speicherverlust aufgetreten.

Ich habe nun alle Wege verfolgt. Es ist kein exit(), kein try-catch-throw oder anderweitiges Element vorhanden, was diese Fehlermeldung rechtfertigen würde.

Ich hab auch die Funktion (ChannelCount()-1) schon gegen ein einfaches (15) ersetzt. Die Fehlermeldung bleibt.

Leider habe ich die Meldung so noch nie gehabt und die MSDN zeigt eher den Fall von "unerreichbarer Code" auf...Solch eine Fehlermeldung erhalte ich aber nicht.

Als man soll ja klare Fragen stellen:
Woran kann es liegen, dass ich den Fehler C4722 erhalte?
Kann es an der Schleife liegen? (Was ich nicht verstehen würde, oder ich hab die absolute Blindheit)

Visual Studio 2008 Professional unter Windows XP und Windows 7.

Danke für euere Gedanken,
Stefan

volkard

Ist Pos unsigned?

otze

Erstmal ist es ne Warnung und kein Fehler.

zweitens kann size_t niemals <0 werden. size_t kennt keine negativen Werte.

stefanjann

otze schrieb:

Erstmal ist es ne Warnung und kein Fehler.

Ich habe gelernt, dass jede Warung einen Sinn hat, sonst würde es diese nicht geben. Und Warnungen irgnorieren liegt mir aus Prinzip nicht. Ich will zumindest wissen, warum es eine Warnung gibt!

otze schrieb:

zweitens kann size_t niemals <0 werden. size_t kennt keine negativen Werte.

Wie kommt ihr zwei auf negative Werte? size_t ist bei mir ein typedef auf unsigned long.

CStoll

Und drittens: Hat es einen tieferen Sinn, daß die Schleife unbedingt rückwärts zählen soll? (mit der manuellen Variante zählst du auch vorwärts)

volkard

stefanjann schrieb:

Wie kommt ihr zwei auf negative Werte? size_t ist bei mir ein typedef auf unsigned long.

Also ist

Pos>=0

immer wahr.

stefanjann

CStoll schrieb:

Und drittens: Hat es einen tieferen Sinn, daß die Schleife unbedingt rückwärts zählen soll? (mit der manuellen Variante zählst du auch vorwärts)

for (size_t Pos(0); Pos<CountSize(); ++Pos)
// In jedem Durchgang wird die Funktion CountSize() abgerufen

for (size_t Pos(CountSize()-1); Pos>=0; --Pos)
// Funktion CountSize wird nur einmal aufgerufen

Also, ja, es ist eigentlich schon absicht.

otze

stefanjann schrieb:

Ich habe gelernt, dass jede Warung einen Sinn hat, sonst würde es diese nicht geben. Und Warnungen irgnorieren liegt mir aus Prinzip nicht. Ich will zumindest wissen, warum es eine Warnung gibt!

Ja, aber wenn man sie nicht versteht, sollte man das Progrmam zumindest mal mit dem Debugger durchlaufen lassen um zu sehen, ob da was falsch ist. Der VC hat was Warnungen angeht nämlich auch ein paar Macken - er warnt bei völlig korrektem, validen C++.

Also, ja, es ist eigentlich schon absicht.

Tue nichts, was der Compiler besser kann als du. Normalerweise ist deine Aussage nämlich falsch, weil der Compiler schlau ist. Und optimiere keine einzelnen Takte in nem Destruktor. Mache da slieber dort, wo du wirklich optimieren musst, weil es für das Programm _relevant_ ist. Wieviel Code hättest du in der Zwischenzeit schreiben können, hättest du nicht "optimiert"?

Ansonsten:

size_t end = CountSize();

stefanjann

volkard schrieb:

stefanjann schrieb:

Wie kommt ihr zwei auf negative Werte? size_t ist bei mir ein typedef auf unsigned long.

Also ist
Pos>=0
immer wahr.

Ja, klar...und folglich hab ich bei Pos=0, --Pos auch sofort wieder nen positiven Wert.

*fluch-auf-mich*

Danke euch.

stefanjann

otze schrieb:

Tue nichts, was der Compiler besser kann als du. Normalerweise ist deine Aussage nämlich falsch, weil der Compiler schlau ist. Und optimiere keine einzelnen Takte in nem Destruktor. Mache da slieber dort, wo du wirklich optimieren musst, weil es für das Programm _relevant_ ist. Wieviel Code hättest du in der Zwischenzeit schreiben können, hättest du nicht "optimiert"?

Ansonsten:
size_t end = CountSize();

Naja, die Funktion ChannelRemoveAll ist ja nicht explizit für den d'tor geschrieben, sondern wird unter anderem im d'tor verwendet. Im Programm wird die nach jedem Song aufgerufen, weil die Channel und Ports sich jedes Lied ändern.

Und wenn mal viele Songs parallel geladen und entladen werden, möchte ich schon so kompackte wie möglich programmiert haben.

Das ein Compiler meinen Code sowieso noch durch nen Optimizer schickt ist mir auch klar. Trotzdem sollte man sich schon gedanken machen, was man schreibt. Und da gehören Erfahrungswerte wie dieser Thread einfach mit dazu. Aus Fehlern lernt man.

stefanjann

otze schrieb:

Ja, aber wenn man sie nicht versteht, sollte man das Progrmam zumindest mal mit dem Debugger durchlaufen lassen um zu sehen, ob da was falsch ist. Der VC hat was Warnungen angeht nämlich auch ein paar Macken - er warnt bei völlig korrektem, validen C++.

Du meinst sicherlich wie sowas wie "Warnung, nicht alle Steuerpfade geben einen Wert zurück" nach einen throw und wenn man ein return einfügt, dann hat man plötzlich unerreichbaren Code?

Ja, habe ich auch schon festgestellt.

CStoll

stefanjann schrieb:

CStoll schrieb:

Und drittens: Hat es einen tieferen Sinn, daß die Schleife unbedingt rückwärts zählen soll? (mit der manuellen Variante zählst du auch vorwärts)
for (size_t Pos(0); Pos<CountSize(); ++Pos)
// In jedem Durchgang wird die Funktion CountSize() abgerufen

for (size_t Pos(CountSize()-1); Pos>=0; --Pos)
// Funktion CountSize wird nur einmal aufgerufen
Also, ja, es ist eigentlich schon absicht.

Wie otze schon schrieb - im Zweifelsfall kannst du den Wert auch einmal berechnen lassen und dann in der Schleife darauf zugreifen. Außerdem glaube ich nicht, daß die Funktion CountSize() so komplex ist, daß sie wirklich bei jedem Aufruf die 16 Kanäle zählen muß. (d.h. durch Inlining und Optimierungen fliegt der Funktionsaufruf komplett weg)

volkard

Rückwärts zu löschen ist generell gut, wenn vorwärts angelegt wird.
Leider kann man dann nicht mehr so einfach eine for-Schleife mehr schreiben.
Ich könnte mir aber vorstellen, daß der Compiler damit keine Probleme hat:

for (size_t Count(CountSize()); Count>0; --Count)
   lösche(Count-1);

Wobei das fast riecht, wie

//Wenn ich groß bin, werde ich ein vector
while(CountSize())
   lösche(Count-1);

stefanjann

volkard schrieb:

Rückwärts zu löschen ist generell gut, wenn vorwärts angelegt wird.
Leider kann man dann nicht mehr so einfach eine for-Schleife mehr schreiben.
Ich könnte mir aber vorstellen, daß der Compiler damit keine Probleme hat:
for (size_t Count(CountSize()); Count>0; --Count)
   lösche(Count-1);
Wobei das fast riecht, wie
//Wenn ich groß bin, werde ich ein vector
while(CountSize())
   lösche(Count-1);

Hallo,
ja es it ein Vector. Allerdings macht mein Lösch-Befehl den Vector nicht kleiner. Da sonst sehr viele insert und resize benötigt würden. Die Funktion zum ChannelRemoveAll steht oft zur Verfügung und deswegen wird nur das Element darin gelöscht und der Zeiger auf 0 gesetzt.
Es ist zwar eine Midi-Klasse, ich nutze diese aber auch für einen "virtuellen Midiverwalter" und der kann size_t Channel haben (nicht nur die 16 Standard).
Bei großen Songs kann dann im Lied (wenn die Mappings sich ändern) schon einiges an Rechenleistung vergehen. Daher geht es mit dem "Geruch" leider nicht.

Aber die erste Idee ist gut.

Danke.

otze

Da sonst sehr viele insert und resize benötigt würden.

und? Was meinste, wie der vector resize implementiert? ein resize gibt normalerweise keinen Speicher mehr frei. Da heißt, wenn du den Vector kleiner machst, dann kostet dich das niemals etwas. musst du halt vorher mit reserve arbeiten. insert in der Mitte brauchst du wohl auch nie, also ist auch das eine O(1) Operation.

Eventuell sollte man dir sagen, dass iterationen gar nichts kosten. Und ich wette, dass du irgendwo Fehler machst, die sich 10000x mal stärker auf die performance auswirken.

Hast du überhaupt mal gemessen, oder glaubst du einfach nur, dass das auf die Performance geht? Ich würde einfahc mal vermuten, dass im Vergleich zum performance kritischen Teil (datei einlesen, verarbeiten, abspielen) dein destructor vielleicht 0.1% der Zeit einnimmt.

Merke: erst messen, dann optimieren. Lesbarer Code ist immer besser als getrickster Code. Lesbarer Code wird häufig auch besser vom Compiler für dich optimiert. Trickse also nur dort, wo es notwendig ist.