C-struct Array effektiv nach C++ konvertieren

[Klaus M. (klaus_me)]

Hallo, ich habe in einem Programm eine "alte" C-Struktur. Da ich die GUI 
inzwischen mit Qt programmiere wäre es wohl sinnvoll, auch in 
Datenstrukturen nach und nach auf C++ umzustellen. Es handelt sich um 
ein Tool für Schlagzeuger.
Eine wichtige Struktur sind die Drum-Pattern. Die Drum-Pattern werden 
aus einer Text-Datei eingelesen und sind im Prinzip MIDI-Noten in 
Textform.

Diese sehen z.B. so aus:

P Paradiddle16-2
T 21 1
C 4 4 28 8 90
S 8 S6:4 S6:4 S6:4 S6:4 S6:4 S6:4 S6:4 S6:4 
N 0 36 64 
N 0 42 64 
N 240 42 64 
N 240 36 64 
N 480 38 100 
N 480 42 64 
N 720 42 64 
N 960 42 64 
N 960 36 64 
N 1200 42 64 
N 1440 38 90 
N 1440 42 64 
N 1680 42 64 
N 1920 36 64 
N 1920 42 64 
N 2160 42 64 
N 2160 36 64 
N 2400 38 70 
N 2400 42 64 
N 2640 42 64 
N 2880 43 104R
N 3000 38 70l
N 3120 47 64r
N 3240 47 64r
N 3360 38 100L
N 3480 48 64r
N 3600 38 70l
N 3720 38 70l
E

Insgesammt sind weit über 1000 Schlagzeug-Pattern in der Textdatei. 
Diese werden beim Programmstart eingelesen und in eine C-Struktur
umgewandelt.

// Zusammengestuzt...
#define MAX_NAME (32-8)
#define MAX_STEPS 64
#define MAX_PATTERN 2500

struct Note {
    uint16_t time;
    uint8_t note, vel;
    uint8_t flags, stick;
};

struct DrumPat {
    uint8_t genre, type;
    uint8_t num, denum;
    uint8_t bpm, max_beats;
    uint8_t num_notes, num_step_entries;
    // Offset name: 8 bytes
    char name[MAX_NAME];
    // Offset steps 32 bytes
    int8_t steptable[MAX_STEPS];
    // Offset notes: 96 bytes
    struct Note notes[];    
};    

struct DrumPat *drumpattern[MAX_PATTERN];

size_t getPatternSize(int32_t num_notes)
{
    // Calculate size of drum pattern
    return (sizeof(struct DrumPat) + (num_notes+1) * sizeof(struct Note));
}

Die Einleseroutine berechnet die Größe jeder Struktur (hängt von der 
Anzahl der Noten ab) und alloziert den Speicher per malloc. Dabei sind 
einige Größen fest codiert, was nicht so schön ist.
Nun würde ich gerne das ganze in einen sinnvollen C++ Container 
überführen, also eine Art std::list in der dann die einzelnen Elemente
jeweils ein Schlagzeug Pattern darstellt, wobei dort dann sowohl der 
Name, StepTable und Noten dynamische Arrays sind. Wobei zum bloßen 
speichern würde auch ein std::array gehen und kein std::vector. Bisher 
ist es so, wenn ich ein pattern verändere, alloziere ich Speicher für 
ein neues Pattern und geben dann den alten Speicher frei und weise dem 
passenden Eintrag der Zeiger *drumpattern[n]; die neue Adresse zu.

Nun kann ich einigermaßen C aber nur wenig C++. Was wäre hier eine 
sinnvolle Vorgehensweise?

[Udo K. (udok)]

Klaus M. schrieb:
> Nun kann ich einigermaßen C aber nur wenig C++. Was wäre hier eine
> sinnvolle Vorgehensweise?

Sinnvoll ist es, das so zu lassen wie es ist, und die Zeit in Features 
zu stecken von denen der Anwender was hat.

Wenn dir langweilig ist und du unübersichtliche Syntax magst, dann 
kannst du natürlich sowas hinschreiben wie:

std::vector<DrumPat> drumpattern;

struct DrumPat {
    uint8_t genre, type;
    uint8_t num, denum;
    uint8_t bpm, max_beats;
    uint8_t num_notes, num_step_entries;
    std::string name;
    std::vector<int8_t> steptable;
    std::vector<Note> notes;    
};

Für die Laufzeit und den Speicherverbrauch wird es keinen merkbaren 
Unterschied machen.

[Frank D. (Firma: LAPD) (frank_s634)]

Klaus M. schrieb:
> Nun kann ich einigermaßen C aber nur wenig C++. Was wäre hier eine
> sinnvolle Vorgehensweise?

Es so zu lassen wie es ist.

[Klaus M. (klaus_me)]

Das ist alles? Hintergrund: Ich habe mit doch etliche bugs gekämpft, mal 
ein Indizes um eines zu hoch uns schon schlägt der AdressSanitizer zu.
Der hat auch Bugs gefunden, die mir noch gar nicht aufgefallen sind ;-)
Auch in die Beschränkung auf 26 Zeichen beim Namen inzwischen nervig.
Viele Pattern haben nur 4-8 Einträge bei der StepTable, bei manchen 
reichen aber die 64 nicht...

Die Felder "uint8_t num_notes, num_step_entries" bräuchte ich auch nicht 
mehr, da ich diese Infos über pat.notes.size() bekomme, oder?

Ich mach dann nur noch ein "new struct DrumPat pat" und C++ kümmert sich 
um die dynamischen Allozierung von std::string name, std::vector 
steptable und std:vector notes?

Und dann einfach beim einlesen ein

std::vector<DrumPat> drumpattern;
...
while (drumpatterin in textfile)
{
  new struct DrumPat pat; // New -> malloc
  // pat befüllen
  pat.genre = genre;
  pat.type = type;
  ...
  while (steps in patternfile)
    pat.steptable.push_back(step);
  while (notes in patternfile)
    pat.notes.push_back(note);
  ...
}
drumpattern.push_back(pat);

Zum zeichnen der Noten dann

std::list<DrumPat>::iterator actpat;
std::list<Note>::iterator note;
// actpat auf das aktuell zu zeichnende pattern legen
...
for (std::vector<Note> note 
for (note : actpat.notes)
{
  draw_note(note.time, note.note, note.vel, ...)
}

Frank D. schrieb:
> Es so zu lassen wie es ist.

Ok, ich dachte nur, die C-Struktur ist etwas tricky, C++ wäre 
"sauberer".
Vom Speicher dürfte ich nicht viel gewinnen, da ja ein std::vector ein 
paar Zeiger speichert, jeder Zeiger sind 8 Bytes auf einem 64 Bit 
System.

Also eher C lassen? Hätte auch den Vorteil, sollte ich das Tool doch mal 
wieder auf einem embedded ARM portieren, dürfte das (etwas) einfacher 
sein.
Meine größten Probleme könnte ich mit Zeigern lösen, in plain C:

struct DrumPatX {
    uint8_t genre, type;
    uint8_t num, denum;
    uint8_t bpm, max_beats;
    uint8_t num_notes, num_step_entries;
    char *name;
    int8_t *steptable;
    // offset notes: 24 bytes on 64-bit-systems
    // offset notes: 16 bytes on 32-bit-systems
    struct Note notes[];    
};

Eine Sortierfunktion sollte auch in C-Only gut machbar sein, da ich
ja direkt die Zeiger

struct DrumPat *drumpattern[MAX_PATTERN];

manipulieren kann.

[Hans-Georg L. (h-g-l)]

Klaus M. schrieb:
> Hallo, ich habe in einem Programm eine "alte" C-Struktur.
snip

> Bisher ist es so, wenn ich ein pattern verändere, alloziere ich Speicher
> für ein neues Pattern und geben dann den alten Speicher frei
> Nun kann ich einigermaßen C aber nur wenig C++. Was wäre hier eine
> sinnvolle Vorgehensweise?

Sobald du deine C struct mit einem C++ Compiler übersetzt verhält sie 
sich wie eine Klasse. Hat also Konstruktoren und Destruktor. Der einzig 
wichtige Unterschied zu einer Klasse ist, das bei der struct alles 
public ist
im Gegensatz zu einer Klasse da ist alles private. Da könntest du zum 
Beispiel deine Parameter einem Konstruktor übergeben und mit new und 
delete deine Struktur anlegen und löschen. Du könntest auch eine Methode 
play_note(uint98_t note)implementieren usw. Ich denke das würde schon 
einiges
vereinfachen ...

[Oliver S. (oliverso)]

Prinzipiell hast du schon recht. Ein sauberes C++-Programm sollte kein 
new uns schon gar kein malloc enthalten. Das überlässt man alles den 
Containern.

Der richtige Ansatz ist über std::vector, wie oben schon gezeigt.

Oliver

[Udo K. (udok)]

Klaus M. schrieb:
> Die Felder "uint8_t num_notes, num_step_entries" bräuchte ich auch nicht
> mehr, da ich diese Infos über pat.notes.size() bekomme, oder?
>
> Ich mach dann nur noch ein "new struct DrumPat pat" und C++ kümmert sich
> um die dynamischen Allozierung von std::string name, std::vector
> steptable und std:vector notes?
> ...

Wenn du "new std::vector<DrumPat\*>" wie in deinem Beispiel verwendest, 
hast du keinen Vorteil, ausser das jetzt malloc "new" heisst und du nur 
3 Buchstaben tippen musst (aber delete gleicht das wieder aus).
Ich habe nicht umsonst std::vector<DrumPat> (ohne\*) vorgeschlagen. 
Damit kümmert sich der std::vector um die Speicherverwaltung.  Du 
brauchst aber noch einen Konstruktor DrumPat::DrumPat(), einen 
Destruktor DrumPat::~DrumPat(), einen Kopy Konstruktor 
DrumPat::DrumPat(const DrumPat&), und neuerdings einen Move Konstruktor 
operator=(DrumPat&&) -- du kannst dich aber eventuell auch auf die 
automatisch vom Compiler generierten Funktionen verlassen.  Und plane 
ein paar Wochen zum Verplempern ein, um rauszufinden wann diese 
Funktionen aufgerufen werden, und was genau der feine Unterschied 
zwischen & und && ist, und ob nun Referenzen besser sind als Zeiger... 
und warum die Fehlermeldungen plötzlich 5 Zeilen lang sind, und achte 
genau auf das "const", das macht noch einen feinen Unterschied.  In C++ 
gibt es X Wege zum Ziel, wo es in C genau einen Weg gibt.  Diese 
Entscheidungen zu treffen kostet dem Gelegenheitsprogrammierer viel 
wertvolle Zeit, wie man schön an deiner ersten Frage hier sieht.

[Oliver S. (oliverso)]

Udo K. schrieb:
> Wenn du "new std::vector<DrumPat\*>" wie in deinem Beispiel verwendest,
> hast du keinen Vorteil, ausser das jetzt malloc "new" heisst und du nur
> 3 Buchstaben tippen musst (aber delete gleicht das wieder aus).
> Ich habe nicht umsonst std::vector<DrumPat> (ohne\*) vorgeschlagen.
> Damit kümmert sich der std::vector um die Speicherverwaltung.  Du
> brauchst aber noch einen Konstruktor DrumPat::DrumPat(), einen
> Destruktor DrumPat::~DrumPat(), einen Kopy Konstruktor
> DrumPat::DrumPat(const DrumPat&), und neuerdings einen Move Konstruktor
> operator=(DrumPat&&) -- du kannst dich aber eventuell auch auf die
> automatisch vom Compiler generierten Funktionen verlassen.

Zum Glück legt der Compiler bei einfachen structs die benötigten 
Operatoren als default an. Die Konstrktoren bekommt man dazu sicher hin.

Udo K. schrieb:
> std::vector<Note> notes;

std::string sollte da besser geeignet sein.

Oliver

[Bruno V. (bruno_v)]

Am Ende kocht C++ auch nur mit Wasser. Wenn Du die Speicherverwaltung 
C++ überlässt, hast Du auf einem PC vermutlich die geringsten Probleme, 
weil Speicher keine Rolle spielt und Du Dich um nichts kümmern musst. 
Selbst Speicherlecks sind kein Problem, solange sie nur durch 
Benutzer-Interaktion entstehen.

Prinzipiell kannst Du auch in C alle Elemente dynamisch machen. Mit ein 
paar Zugriffs-Funktionen ist es ähnlich wie in C++.

Es macht nur auf einem PC mit unlimitiertem Speicher keinen Sinn.

Sinnvoll wäre das nur, wenn Du in einem Stück Ram zu Beginn möglichst 
viele Strukturen total unterschiedlicher Größe einlesen möchtest und zur 
Laufzeit nicht wieder freigibst. Oder direkt im (limitierten) ROM.

[Oliver S. (oliverso)]

Bruno V. schrieb:
> Am Ende kocht C++ auch nur mit Wasser. Wenn Du die
> Speicherverwaltung
> C++ überlässt, hast Du auf einem PC vermutlich die geringsten Probleme,
> weil Speicher keine Rolle spielt und Du Dich um nichts kümmern musst.

Die Structs sind in C++ genauso groß wie in C, und das bisschen 
Verwaltung für den Vektor spielt kaum eine Rolle.


> Selbst Speicherlecks sind kein Problem, solange sie nur durch
> Benutzer-Interaktion entstehen.

Der Vorteil der Standard Container ist, daß du damit keine Speicherlecks 
bekommst.


>
> Prinzipiell kannst Du auch in C alle Elemente dynamisch machen. Mit ein
> paar Zugriffs-Funktionen ist es ähnlich wie in C++.
>
> Es macht nur auf einem PC mit unlimitiertem Speicher keinen Sinn.

Es macht darauf ganz genauso viel Sinn wie auf Rechnern mit limitiertem 
Speicher.

Oliver

[Udo K. (udok)]

Oliver S. schrieb:
> Die Structs sind in C++ genauso groß wie in C, und das bisschen
> Verwaltung für den Vektor spielt kaum eine Rolle.
Nein, sie sind grösser.  Er braucht ja zwei std::vector (etwa 32 Bytes 
pro vector) und einen std::string (auch etwa 32 Bytes) für sein struct 
DrumPat.
Dazu allokiert jeder vector und string intern Bufferspeicher damit er 
nicht für jeden Furz ein malloc machen muss.

>> Benutzer-Interaktion entstehen.
>
> Der Vorteil der Standard Container ist, daß du damit keine Speicherlecks
> bekommst.

Der TE hat in seinem C++ Beispiel mit std::vector schon ein Speicherleak 
drinnen...  wenn man C++ nicht wirklich versteht ist die Gefahr für 
Fehler gross.  C++ mit den ganzen Libs dazu ist ziemlich komplex.

[Oliver S. (oliverso)]

Udo K. schrieb:
> C++ mit den ganzen Libs dazu ist ziemlich komplex.

Noch viel komplexer ist handgedenglter C-Code, der die gleiche 
Funktionalität nachbilden will.

Wenn man sich allerdings an

Oliver S. schrieb:
> hast du schon recht. Ein sauberes C++-Programm sollte
> kein
> new uns schon gar kein malloc enthalten. Das überlässt man alles den
> Containern.

hält, klappt es auch ohne Memory leaks.

Oliver

[Daniel A. (daniel-a)]

Udo K. schrieb:
> Ich habe nicht umsonst std::vector<DrumPat> (ohne\*) vorgeschlagen.
> Damit kümmert sich der std::vector um die Speicherverwaltung.

Wir wissen ja nicht, ob es noch weitere Pointer auf das DrumPat gibt. Da 
wäre es dann unpraktisch, wenn std::vector die Instanzen verschiebt.

[Klaus M. (klaus_me)]

Udo K. schrieb:
> Der TE hat in seinem C++ Beispiel mit std::vector schon ein Speicherleak
> drinnen...

Du meinst das "new struct DrumPat pat;", oder?
Wir war (noch) nicht klar, dass push_back() eine Kopie anlegt und
der Speicher dazu automatisch alloziert wird. Wobei das bei wenig 
nachdenken ja logisch ist, ist ja bei der steptable und notes auch
so. Es reicht also nur eine struct auf dem Stack die immer wieder
neu befüllt wird.

std::vector<DrumPat> drumpattern;
struct DrumPat pat;
...
while (drumpattern in textfile)
{
  // pat befüllen

  pat.genre = genre;
  pat.type = type;
  ...
  while (steps in patternfile)
    pat.steptable.push_back(step);
  while (notes in patternfile)
    pat.notes.push_back(note);
  ...
  drumpattern.push_back(pat);
}