Statische Funktion-scoped Pointer und Speicher Lecks

https://stackoverflow.com/questions/2136424

22-09-2019
|

Frage

Ich habe geschrieben eine einfache Bibliothek-Datei ist mit einer Funktion zum Lesen von Zeilen aus einer Datei beliebiger Größe.Die Funktion wird aufgerufen, indem in einem stack-allocated buffer und Größe, aber wenn die Linie ist zu groß, eine spezielle heap-zugewiesenen Puffer initialisiert und verwendet, um den pass wieder eine größere Linie.

Dieser heap-zugewiesenen Puffer übergeben wird der Funktion-Bereich und als static deklariert, initialisiert auf NULL am Anfang natürlich.Ich habe geschrieben in einige der Kontrollen an den Anfang der Funktion, um zu prüfen, ob die heap-buffer non-null;wenn dies der Fall ist, dann die Vorherige Zeile Lesen zu lang war.Natürlich habe ich freie heap-Puffer und legen Sie Sie zurück auf NULL, zu denken, dass der nächste gelesen werden wahrscheinlich müssen nur füllen die stack-allocated buffer (es sollte sehr selten zu sehen, Linien, die über 1MB lange, auch in unserer Anwendung!).

Ich habe mich über den code getestet und es ziemlich gründlich, sowohl durch das Lesen Sie es sorgfältig durch und durch ausführen ein paar tests.Ich bin relativ sicher, dass die folgenden Invarianten beibehalten wird:

Die heap-buffer-null (und wird nicht Leck Speicher) auf die Funktion zurück, wenn der stack-Puffer ist alles, was benötigt wird.
Wenn die heap-Puffer ist nicht null, weil es notwendig war, wird es freigegeben werden, auf dem nächsten Aufruf der Funktion (und ggf. wieder verwendet werden, wenn nötig, auf die nächste Zeile).

Aber ich habe gedacht, dass ein potentielles problem:Wenn die Letzte Zeile in einer Datei zu lang ist, da die Funktion ist vermutlich nicht wieder angerufen, ich bin mir nicht sicher, ob ich irgendeine Möglichkeit, um kostenlos die heap-buffer-es ist Funktion-Bereich, nachdem alle.

Also meine Frage ist, wie gehe ich über die Befreiung von dynamisch zugewiesenen Speicher in einer Funktion-Bereich in statische Zeiger, im Idealfall ohne den Aufruf der Funktion wieder?(Und im Idealfall, ohne dass es eine Globale variable, entweder!)

Code auf Anfrage erhältlich.(Ich habe Sie nicht bekommen, access now, sorry.Und ich hoffe, die Frage ist allgemein genug und gut erklärt für die es nicht erforderlich ist, aber von allen bedeutet, zögern Sie befreien mich von dieser Vorstellung!)

EDIT:Ich habe das Gefühl, ich sollte hinzufügen, ein paar Notizen über die Verwendung der Funktion.

Diese Besondere Funktion wird in form von Linien Lesen Sie abwechselnd die aus einer Datei, und dann sofort kopiert, in POD-Strukturen, eine Zeile pro struct.Diese werden auf dem heap erstellt, wie die Datei gelesen wird, und jede dieser Strukturen hat einen char-Zeiger enthält (ein aufgeräumter version) eine Zeile aus der Datei.Um diese zu bestehen, sind eine Kopie der bereits zu erfolgen hat.(Das war eine der großen Gegenargumente gebracht in viele Antworten-- ach Nein, die Linie KOPIERT werden muss, oh dearie me).

Wie für multithreading, als ich sagte, es ist entwickelt, um verwendet werden, die Seriell.Nein, es ist nicht thread-sicher, aber das ist mir egal.

Danke für die Vielzahl an Antworten, aber!Ich werde Lesen Sie mehr gründlich, wenn ich die Zeit.Derzeit, ich bin Neigung in Richtung entweder die übergabe eines zusätzlichen Zeigers um-oder Neugestaltung der Funktion, so dass, wenn fgets zeigt EOF, dann könnte ich nur auf die Befreiung der Logik gibt es statt und die Benutzer hoffentlich nicht brauchen, um sorgen über es.

Lösung

Wenn Sie die Funktion ändern können, würde ich empfehlen, die Funktion Schnittstelle selbst zu verändern. Ich weiß, Sie haben viel Zeit Debuggen ausgegeben und testen, aber es gibt ein paar Probleme mit der aktuellen Implementierung:

es ist nicht Thread-sicher,
der Benutzer keine Kontrolle über die Daten hat, so dass er es kopieren muss, wenn er es später braucht, höchstwahrscheinlich in einem Puffer, der malloc()ed sein wird, um somit einen Vorteil zunichte macht Sie durch den gezielten Einsatz von malloc() in Ihrer Funktion erhalten,
am wichtigsten ist, wie Sie entdeckt haben, eine besondere Maßnahmen ergriffen werden, hat durch den Benutzer für eine lange letzte Zeile.

Der Benutzer sollte nicht durch die Implementierung Kuriosität Ihrer Funktion besorgt werden, sollten sie in der Lage sein, „nur es verwenden“.

Wenn Sie es für pädagogische Zwecke tun, würde ich empfehlen, sich unter diese , die eine Implementierung, und Links zu anderen derartigen Implementierungen haben „eine beliebig lange Linie aus einem Stream zu lesen“ (jede Implementierung ist etwas anders als die anderen, so sollten Sie in der Lage sein, einen zu finden, die Sie mögen) .

Basierend auf Ihrem bearbeiten, MT-Safe ist nicht erforderlich, und eine Kopie wird immer passieren. So ist die naheliegendste Design ist eine der beiden:

Lassen Sie den Benutzer eine char ** liefern, die Punkte auf einen Puffer, dass Ihre Funktion zuweisen, eine Kombination aus malloc() und realloc() (falls erforderlich). Es liegt in der Verantwortung des Anwenders, es zu free() wenn Sie fertig sind. Auf diese Weise muss der Benutzer nicht erneut, die Daten zu kopieren, da er einen Zeiger passieren kann, wo das endgültige Ziel der Daten ist.
zurückgeben char *, die von Ihrer Funktion zugeordnet ist. Wieder ist es die Verantwortung des Anwenders, es zu free().

Beide sind ziemlich gleichwertig.

Für Ihre aktuelle Implementierung können Sie immer wieder zurückkehren „nicht von Datei anzeigen“, wenn die letzte Zeile ist sehr lang, und in einem Newline endet nicht. Dann wird der Benutzer gehen, um Ihre Funktion wieder anrufen, und dann können Sie Ihre Puffer freizugeben. Persönlich würde ich mit einer Funktion glücklicher sein, die mich so viele Zeilen lesen kann, wie ich will, und nicht zwingen, mich bis zum Ende der Datei zu gehen.

Andere Tipps

Abgesehen von der Schwierigkeit, die dynamisch zugewiesenen Puffer freigegeben werden kann, gibt es ein weiteres potenzielles Problem. Es ist nicht Thread-sicher. Da es sich um eine Bibliotheksfunktion ist, dann gibt es immer, dass die Möglichkeit, wird es in einem Multi-Threaded-Umgebung in der Zukunft verwendet werden.

Es wäre wahrscheinlich besser, die anrufende Funktion erfordert die Puffer über eine verbundene Bibliotheksfunktion zu befreien.

Das könnte noch in Ordnung sein, wenn Sie die Standard-Technik verwenden, End-of-Datei anzuzeigen (d haben Sie lesen-line-Funktion return NULL).

Was passiert in diesem Fall ist, dass nach der letzten Zeile gelesen wird, ein weiteren Anruf an dem Lese-line-Funktion wird so erforderlich sein, dass es NULL zurück, um anzuzeigen, dass das Ende der Datei erreicht ist. In diesem letzten Gespräches können Sie dann frei, Sie puffern.

Zwei Entscheidungen, die sofort auftreten:

Machen Sie den Zeiger auf dem Heap zugewiesenen Puffer statisch, sondern Datei scoped. Fügen Sie eine (statische) Funktion, die prüft, ob es nicht null ist und wenn es nicht null ist kostenlos () s es. Call atexit (free_func) zu Beginn des Programms, wo free_func ist die statische Funktion. Sie können einige globale Setup-Routine haben (caled von main ()), wo dies geschehen ist.
Kümmere dich nicht um sie; Heap-Speicher zugewiesen wird durch das OS, wenn der Prozess beendet wird freigegeben, und der Speicherverlust ist nicht kumulativ, so dass selbst wenn Ihr Programm eine lange Lebensdauer hat es eine OOM Ausnahme nicht erhöhen (es sei denn, Sie einige andere Fehler haben).

Ich nehme an, Ihre App nicht multithreaded; in diesem Fall sollten Sie nicht einen statischen Puffer überhaupt verwenden, oder Sie sollen lokale Thread-Daten verwenden.

Die Schnittstelle, die Sie gewählt haben, ist dies ein unlösbares problem:

Der client muss nicht wissen, ob der Rückgabewert zeigt auf statische oder dynamische Speicher.
Der Rückgabewert muss sich auf Speicher, der überlebt den Anruf.
Jeder Anruf könnte der Letzte sein.

Ich bin mir nicht sicher, warum Sie beunruhigt sind durch dieses Leck.Nach allem, wenn der client liest eine sehr lange Zeile, macht das etwas mit der Zeile, dann wird ein ton, der Berechnung und Zuweisung vor dem Lesen der nächsten Zeile, Sie haben noch ein großes Stück Speicher herum sitzt, unbenutzt, verstopfen das system.Wenn das OK mit dir (willkürliche Berechnung erfolgt vor dem Speicher zurückgefordert), können Sie einfach fess up, den Sie bereit sind behalten tot Speicher auf unbestimmte Zeit.

Wenn Sie können nicht Leben mit dem Leck, das einfachste, was zu tun ist, zu erweitern die Schnittstelle, die dem client mitteilen, Ihre Funktion, wenn der client mit der Erinnerung.(Jetzt den Vertrag mit dem Kunden sagt, dass der client besitzt Speicher, bis es ruft die Funktion wieder an dem Punkt das Eigentum wieder Ihre Funktion.) Natürlich, ändern Sie die Schnittstelle bedeutet, dass Sie entweder

hinzufügen einer neuen Funktion, die verlangen, Sie zu fördern, Ihre Zeiger zu static aber die lokalen, die an der Zusammenstellung Einheit, oder
hinzufügen von einigen argument, um die bestehende Funktion (oder überbelastung ein argument), so dass Sie einen Anruf, was bedeutet, "ich bin fertig mit Ihr Gedächtnis jetzt, aber ich will nicht eine andere Linie".

Eine weitere Radikale änderung wäre, zu schreiben, die Funktion zu verwenden, dynamisch zugewiesenen Speicher während der gesamten Lebensdauer allmählich vergrößert den block, bis es so groß ist wie der größte block je gelesen (oder vielleicht aufgerundet auf die nächste Potenz von zwei).Abhängig von der tatsächlichen Fälle dieser Strategie belegen können weniger Adresse Raum als das halten einer großen statischen Puffer.

In jedem Fall bin ich nicht überzeugt, sollten Sie sich Gedanken über das Ecke Fall.Wenn Sie denken, dass dieser Fall Fragen, bitte Bearbeiten Ihre Frage, um uns zu zeigen Beweise.

Statt Funktionsumfang, geben ihm Modul Umfang (zB bei Dateiumfang, sondern statisch, so ist es nicht sichtbar außerhalb der Datei. Eine kleine Funktion hinzufügen, dass frees der Puffer und Verwendung atexit(), um sicherzustellen, dass, bevor das Programm beendet genannt wird. alternative, keine Sorge darüber - ein Leck, das nur einmal passiert, und wird automatisch als das Programm beendet befreit ist nicht besonders schädlich

Ich fühle mich zwar für eine Katastrophe wie ein Rezept für mich, dass die Design-Klänge zu sagen, verpflichtet. Wenn Sie den Puffer freizugeben, gibt es praktisch keine Möglichkeit, selbst erraten, ob es noch in Gebrauch sein könnte. Der Benutzer (scheinbar) hat, um zu verfolgen, wo die Daten zurückgegeben wurden, und kopieren Sie die Daten auf einen neuen Puffer, wenn (und nur dann) Sie einen dynamisch zugewiesen. In einer Umgebung mit Multi-Threading, müssen Sie die internen Zeiger lokalen Thread jede Chance richtig funktioniert überhaupt machen zu müssen. Für den Benutzer kann die Funktion eines von zwei völlig verschiedene Dinge tun - entweder einen Puffer zurück, die im Besitz des Benutzers ist oder Rückkehr einen Puffer, der von der Funktion im Besitz ist, und nur sicher einen weiteren Puffer durch die Zuweisung verwendet werden kann, und das Kopieren der Daten in den anderen Puffer, bevor die Funktion erneut aufgerufen wird.

Es gibt ein paar Hacks ich denken kann, obwohl beide erfordern die statische Erklärung aus der Funktion zu bewegen. Ich kann mir nicht vorstellen, warum das wäre ein Problem sein.

Mit einem GCC Erweiterung ,

static char *buffer;
void use_buffer(size_t n) {
    buffer = realloc(buffer, n);
}
void cleanup_buffer() __attribute__((destructor)) {
    free(buffer);
}

Mit C ++,

static char *buffer;
static class buffer_guard {
    ~buffer_guard() { free(buffer); }
} my_buffer_guard;

Auf jeden Fall kann ich nicht wirklich wie das Design. In C, in der Regel ist der Anrufer verantwortlich für die Zuweisung / Freigabe-Speicher, dass es verwenden muss, auch wenn es von einem Angerufenen in gefüllt ist.

BTW, zu vergleichen mit Glibc des Nicht-Standard- getline . Es verwendet nie statische Speicher.

Ich wollte nur auf Kommentar unter Mark Antwort, aber es kann ein wenig beengt fühlen. Trotzdem diese Antwort ist ein Kommentar auf seiner Antwort im Wesentlichen vor, die ich schnell zu sein, zusätzlich sehr gut finden.)

Es ist nicht nur Ihre Funktion nicht MT-sicher, aber auch ohne Gewinde, die Schnittstelle, es zu benutzen richtig kompliziert. Der Anrufer muss mit dem vorherigen Ergebnis abgeschlossen haben, bevor die Funktion erneut aufrufen. Wenn dieser Code noch im Einsatz von zwei Jahren ab jetzt ist, wird jemand den Kopf kratzen versuchen, es zu verwenden, richtig ... oder noch schlimmer, verwenden Sie es falsch, ohne auch nur darüber nachzudenken. Diese Person könnte sogar sein, das Sie ...

Mark Vorschlag (der Anrufer erfordern den Puffer frei) ist IMHO die vernünftigste. Aber vielleicht haben Sie nicht das Vertrauen malloc und free nicht Ursache der Fragmentierung auf lange Sicht, oder haben einen anderen Grund die statische Pufferlösung zu bevorzugen. In diesem Fall können Sie die statischen Puffer halten für gewöhnliche Länge Linien, ein boolesches Flag definieren, die der statische Puffer zeigt an, ob derzeit damit beschäftigt ist, und zu dokumentieren, dass die folgende Funktion (und nicht free) sollte mit der Adresse des Puffers aufgerufen werden, wenn der Anrufer nicht mehr verwendet es:

char static_buffer[512];
int buffer_busy;

void free_buffer(char *p)
{
  if (p == static_buffer)
  {
     assert(buffer_busy);
     buffer_busy=0;
  }
  else free(p);
}

char *get_line(...)
{
  char *result;
  if (..short line..)
  {
     result = static_buffer;
     assert(!buffer_busy);
     buffer_busy=1;
  }
  else result = malloc(...);
  ...
  return result;
}

Die einzigen Umstände, unter denen die Behauptungen auslösen sind Umstände, unter denen Ihre bisherige Implementierung still schiefgegangen haben würde, und der Aufwand ist sehr gering im Vergleich zu Ihrer bestehenden Lösung (nur die Flagge Makel, und bittet den Anrufer free_buffer zu rufen, wenn er ist nicht fertig, ist sauberer). Wenn die Behauptung in get_line insbesondere Trigger, es bedeutet, dass Sie die dynamische Zuordnung schließlich erforderlich, weil der Anrufer nicht mit einem Puffer zu dem Zeitpunkt beendet werden könnte, wurde er für einen anderen zu fragen.

. Hinweis: das ist immer noch nicht MT-safe

Lizenziert unter: CC-BY-SA mit Zuschreibung

Nicht verbunden mit StackOverflow