Static puntatori a funzione con scope e perdite di memoria

https://stackoverflow.com/questions/2136424

22-09-2019
|

Domanda

Ho scritto un file di libreria semplice con una funzione per la lettura di righe da un file di qualsiasi dimensione. La funzione viene chiamata passando in un buffer e stack allocati, ma se la linea è troppo grande, una speciale heap allocato viene inizializzato e utilizzata per passare indietro una linea più grande.

Questo heap allocato è funzione con ambito e dichiarato statica, inizializzato su NULL all'inizio del corso. Ho scritto in alcuni controlli all'inizio della funzione, per verificare se il buffer di heap non è nullo; se questo è il caso, allora la linea precedente lettura era troppo lunga. Naturalmente, ho liberare il buffer di heap e impostare di nuovo a NULL, pensando che la prossima lettura sarà probabilmente solo bisogno di riempire il buffer di stack-assegnati (che dovrebbe essere molto raro vedere linee oltre 1MB lungo, anche nella nostra applicazione!).

Sono andato sopra il codice e testato in modo abbastanza esauriente, sia leggendo attentamente eseguendo alcuni test. Sono ragionevolmente fiducioso che il seguente invariante è mantenuto:

Il buffer mucchio sarà nulla (e non perderà la memoria) sul ritorno funzione se il buffer di stack è tutto ciò che è necessario.
Se il buffer heap non è nullo, perché era necessario, verrà liberata la chiamata successiva funzione (ed eventualmente riutilizzato se necessario in quella riga successiva).

Ma ho pensato a un potenziale problema: Se l'ultima riga in un file è troppo lungo, quindi dal momento che la funzione è presumibilmente non chiamato di nuovo, io non sono sicuro di avere un modo per liberare la buffer-- mucchio è la funzione di ambito, dopo tutto.

Quindi la mia domanda è: come posso fare per liberare memoria allocata dinamicamente in funzione di un puntatore statica con ambito, idealmente senza chiamare di nuovo la funzione? (E, idealmente, senza farne una variabile globale, sia!)

Codice disponibili su richiesta. (Io non ho avuto accesso ora, mi spiace. E sto sperando che la questione è sufficientemente generale e ben spiegato, per esso non sia necessario, ma con tutti i mezzi non esitate a disilludere me di tale nozione!)

EDIT: sento di dover aggiungere un paio di note sull'uso della funzione

Questa funzione particolare viene utilizzato sotto forma di linee si legge in serie da un file, e poi subito copiato nella struct POD, una linea per struct. Coloro che vengono creati sul mucchio come il file viene letto, e ognuno di questi ha una struct puntatore char contenente (una versione ripulita di) una riga dal file. Affinché questi a persistere, una copia è già a verificarsi. (Questo è stato uno dei grandi controdeduzioni cresciuti in molti dei answers-- oh no, la linea deve essere copiato, oh me cara).

Per quanto riguarda il multithreading, come ho detto questo è stato progettato per essere utilizzato in serie. No, non è thread-safe, ma non mi interessa.

Grazie per la moltitudine di risposte, però! Io li leggo più a fondo quando avrò tempo. Attualmente, sto appoggiato verso o passando un puntatore extra intorno o ridisegnare la funzione in modo che quando fgets mostra EOF, allora potrei solo creare la logica liberazione lì invece e l'utente si spera non avrà bisogno di preoccuparsi.

Soluzione

Se è possibile modificare la funzione, mi sento di raccomandare la modifica della funzione stessa interfaccia. So che hai speso un sacco di tempo e di debug testarlo, ma ci sono alcuni problemi con l'implementazione corrente:

non è thread-safe,
l'utente non ha alcun controllo sui dati, quindi deve copiarlo se ne ha bisogno in seguito, molto probabilmente in un buffer che sta per essere malloc()ed, annullando così ogni vantaggio che hai con l'uso selettivo di malloc() nella funzione,
la cosa più importante, come avete scoperto, deve essere presa un'azione speciale da parte dell'utente per un lungo all'ultima riga.

Gli utenti non dovrebbero essere preoccupati dalla stranezza implementazione della funzione, dovrebbero essere in grado di "basta usarlo".

A meno che non si sta facendo per scopi didattici, mi sento di raccomandare guardando questo pagina , che ha un'implementazione di "leggere una linea arbitrariamente lungo da un flusso", e link ad altri tali implementazioni (ogni implementazione è leggermente diverso dagli altri, così si dovrebbe essere in grado di trovare uno che ti piace) .

In base alla tua modifica, MT-safe non è un requisito, e una copia sta andando sempre accadere. Così, il design più evidente è uno dei due:

consentono all'utente di fornire un char **, che punta a un buffer che la funzione assegnerà, utilizzando una combinazione di malloc() e realloc() (se necessario). È responsabilità dell'utente per free() quando fatto. In questo modo, l'utente non deve copiare nuovamente i dati, dal momento che può passare un puntatore ovunque la destinazione finale dei dati è.
restituire un char * che viene allocato per la vostra funzione. Anche in questo caso, è responsabilità dell'utente per free() esso.

Entrambi sono praticamente equivalenti.

Per la vostra implementazione corrente, si può sempre tornare "non finisce di file" se l'ultima riga è molto lunga, e non si esaurisce in una nuova riga. Quindi, l'utente sta per chiamare di nuovo la funzione, e quindi è possibile liberare il buffer. Personalmente, sarei più felice con una funzione che mi permette di leggere il maggior numero di linee come voglio, e non costringermi ad andare alla fine del file.

Altri suggerimenti

A parte la difficoltà di liberare tale buffer allocato dinamicamente, c'è un altro problema potenziale. Non è thread-safe. Poiché si tratta di una funzione di libreria, allora c'è sempre la possibilità che possa essere utilizzato in un ambiente multi-thread in futuro.

Probabilmente sarebbe meglio per richiedere la funzione di chiamata di liberare il buffer tramite una funzione di libreria correlato.

Questo potrebbe essere ancora bene se si utilizza la tecnica standard per indicare end-of-file (cioè Hai letto-line funzione di ritorno NULL).

Quello che succede in questo caso è che, dopo la riga finale viene letta, sarà necessaria una chiamata più per la tua funzione di lettura-line in modo che possa tornare NULL per indicare che il fine del file è stata raggiunta. In questa ultima chiamata, è possibile quindi liberarvi buffer.

Due scelte che si verificano immediatamente:

Fare il puntatore al buffer di heap-assegnati statica, ma file di scope. Aggiungere una funzione (statico) che controlla se non è nullo e se non è nullo free () è tutto. Chiamare atexit (free_func) all'inizio del programma, in cui free_func è la funzione statica. Si può avere un po 'di routine di installazione globale (caled da main ()), dove questo viene fatto.
Non ti preoccupare; memoria heap allocata viene rilasciato dal sistema operativo quando le vostre uscite di processo, e la perdita di memoria non è cumulativo, quindi, anche se il programma ha una lunga vita non solleverà un'eccezione OOM (a meno che non hai qualche altro bug).

Presumo vostra applicazione non è multithread; in questo caso, non si dovrebbe usare un buffer statico a tutti, o si dovrebbe utilizzare i dati thread-locale.

L'interfaccia si è scelto rende questo un problema irrisolvibile:

Il cliente non deve sapere se i punti di valore restituito a memoria statica o dinamica.
Il valore restituito deve puntare a memoria che sopravvive alla chiamata.
Ogni chiamata potrebbe essere l'ultimo.

Non sono sicuro che il motivo per cui si è turbato da questa perdita. Dopo tutto, se il client legge una linea molto lunga, fa qualcosa con la linea, poi fa un sacco di calcolo e di assegnazione prima di leggere la riga successiva, si ha ancora un grande pezzo di memoria seduti attorno inutilizzata, intasando il sistema. Se questo OK con voi (calcolo arbitrario avviene prima che la memoria venga recuperato), si può solo fess up che siete disposti a conservare la memoria morta a tempo indeterminato.

Se non si può vivere con la perdita, la cosa più semplice da fare è di ampliare l'interfaccia in modo che il cliente può comunicare la funzione quando il client è fatto con la memoria. (In questo momento il contratto con il cliente dice che il cliente possiede la memoria fino a quando non chiama di nuovo la funzione, a quel punto la proprietà torna alla funzione.) Naturalmente, per modificare l'interfaccia indica sia

l'aggiunta di una nuova funzione, che richiederebbe di promuovere il puntatore da static ma locale per l'unità di compilazione, o
l'aggiunta di qualche argomento alla funzione esistente (o sovraccarico di un argomento) in modo da avere una chiamata che significa "mi sono fatto con la memoria ora, ma non voglio un'altra linea".

Una modifica più radicale sarebbe riscrivere la funzione da utilizzare memoria allocata dinamicamente tutta la sua durata, ampliando gradualmente il blocco come necessario fino a quando è grande come il blocco più grande mai leggere (o forse arrotondato alla successiva potenza di due ). A seconda di casi concreti di questa strategia può consumare meno spazio di indirizzamento di mantenere un buffer statico grande.

In ogni caso io non sono convinto che si dovrebbe preoccuparsi di questo caso angolo. Se pensi che questo caso le cose, si prega di modificare la tua domanda per mostrarci le prove.

Invece di portata funzione, dare modulo portata (vale a dire nell'ambito di file, ma statico, quindi non è visibile al di fuori quel file. Aggiungere una piccola funzione che libera il buffer, e utilizzare atexit() per assicurare che si chiama prima il programma termina. Alternative, non ti preoccupare - una perdita che avviene solo una volta, e viene liberato automaticamente quando il programma viene chiuso non è particolarmente dannoso

Mi sento in dovere di dire che il design suona per me come una ricetta per il disastro però. Quando liberi il buffer, non c'è praticamente alcun modo di indovinare anche se potrebbe essere ancora in uso. L'utente (apparentemente) deve tenere traccia di dove è stato restituito i dati, e copiare i dati in un nuovo buffer se (e solo se) si assegnata una in modo dinamico. In un ambiente multi-threading, è necessario effettuare il puntatore interno filo-locale per avere qualche possibilità di funzionare correttamente a tutti. Per l'utente, la funzione potrebbe fare una delle due cose completamente diverse - o tornare un buffer che è di proprietà dell'utente, o restituire un tampone che è di proprietà dalla funzione, e può essere utilizzato solo in modo sicuro assegnando un altro buffer, e la copia del dati nell'altro buffer prima che la funzione viene chiamata nuovamente.

Ci sono alcuni hack mi vengono in mente, anche se entrambi richiedono spostando la dichiarazione statica fuori della funzione. Non riesco a immaginare il motivo che sarebbe stato un problema.

Utilizzo di un estensione GCC ,

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

Utilizzando C ++,

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

In ogni caso, non mi piace molto il design. In C, di solito il chiamante è responsabile per l'allocazione / liberazione della memoria che ha bisogno di usare, anche se è riempito da un chiamato.

A proposito, confrontare con la getline . Non è mai utilizza la memoria statica.

Stavo solo andando a commentare qui sotto la risposta di Mark, ma si può sentire un po 'po' angusta. Eppure, questa risposta è in sostanza un commento sulla sua risposta, che trovo molto bene, oltre ad essere rapida:.)

Non solo è la vostra funzione non MT-safe, ma anche senza fili, l'interfaccia da utilizzare in modo corretto è complicata. Il chiamante deve aver finito con il risultato precedente prima di chiamare nuovamente la funzione. Se questo codice è ancora in uso due anni da oggi, qualcuno grattarsi la testa cercando di utilizzare nel modo giusto ... o peggio, lo uso sbagliato, senza nemmeno pensarci. Quella persona potrebbe anche essere voi ...

Il suggerimento di Marco (che richiede al chiamante di liberare il buffer) è IMHO la più ragionevole. Ma forse non ti fidi malloc e free non causare la frammentazione nel lungo periodo, o avere qualche altro motivo per preferire la soluzione tampone statica. In questo caso è possibile mantenere il buffer statico per le linee ordinarie di lunghezza, definire un flag booleano che indica se il buffer statico è attualmente occupato, e documentare che la seguente funzione (e non free) dovrebbe essere chiamato con l'indirizzo del buffer quando il chiamante non è più lo utilizza:

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;
}

Le uniche circostanze in cui le affermazioni saranno innescare sono circostanze in cui l'implementazione precedente avrebbe silenziosamente andato storto, e l'overhead è molto basso rispetto alla soluzione esistente (solo commutando la bandiera, e chiedendo al chiamante di chiamare free_buffer quando ha finito, che è più pulita). Se l'affermazione in get_line in particolare trigger, significa che è necessario l'allocazione dinamica, dopo tutto, perché il chiamante non ha potuto essere rifinito con un buffer nel momento in cui è stato chiesto per un altro.

. Nota: questo non è ancora MT-safe

Autorizzato sotto: CC-BY-SA insieme a attribuzione

Non affiliato a StackOverflow