[C] Stringhe allocate dinamicamente e uso di puts()

[Alex'87]

Originariamente inviato da ramy89
P.S.: ma se io sono sicuro che la memoria verrà allocata,per convenzione devo comunque verificare che non punti a NULL?

Come fai ad essere così sicuro che l'allocazione avvenga correttamente? Non hai alcuna garanzia di successo: puoi non avere problemi mille volte di fila ma magari alla mille+1-esima la malloc fallisce e il programma crasha... È buona norma controllare il successo o meno di ogni operazione "a rischio".

[Andrea Simonassi]

Ciao.

Chiedo scusa a tutti per la lungaggine.

Ho provato a compilare il tuo codice e ho trovato un bel po di errori.

Ecco sosa hai scritto.

#include <stdio.h>
#include <stdlib.h>
#include <string.h>


char* get_string(FILE *fp, char*ptr)
{
	int i=1;
    char ci;//non è inizializzata...

    ptr = (char*)malloc(sizeof(char));    //non verifichi che ptr sia null..
    fflush(stdin);//non serve a niente
    while(ci!=10) //dovresti avere letto il primo carattere prima.
    {
        ci=fgetc(fp);    //il file potrebbe essere finito... devi verificare che ci!=EOF
        ptr[i-1]=ci;		//dovresti assegnarlo solo se ci != 10...
        ptr=(char*)realloc(ptr,i*sizeof(char));    
//dovresti riallocare solo se ci != 10... a meno che non voglia includere 10 nella stringa  
//inoltre stai allocando un byte meno di quanto usato in precedenza, quindi potresti perdere 
//il carattere scritto
//inoltre devi verificare che ptr sia diverso da null
//se lo fosse dovresti curarti di chiamare free sul valore precedente di ptr
//ma tu lo perdi
		i++; //i++ va prima di realloc oppure realloc i+1 non realloc i.
    }
    ptr[i-2]='\0';
    puts(ptr);//una funzione di libreria non deve stampare
    printf("%d",strlen(ptr));//come sopra
    fflush(stdin);//non serve
    return ptr;
}

per prima cosa eliminiamo il puntatore passato come parametro ed il flush sullo stdin che non ci interessa dal momento che non stai lavorando su stdin (per quanto ne sappia la tua funzione)
al limite sarà il chiamante a usarlo, eliminiamo printf e puts, la funzione deve solo restituire una stringa
non deve avere effetti collaterali, controlliamo anche il raggiungimento della fine del file (ci!= EOF),
inizializziamo ci per la prima iterazione, correggiamo la dimensione della riallocazione, devi allocare i+1
non i,

char* get_string_1(FILE *fp)
{
   char * ptr=(char*)malloc(sizeof(char));
    int i=0;
    char ci = fgetc(fp);
    
    while(ci!=10 && ci!=EOF)
    {        
        ptr[i++]=ci;		
        ptr=(char*)realloc(ptr,(i+1)*sizeof(char));        
	ci=fgetc(fp);
    }
    ptr[i]='\0';
    return ptr;
}

seconda cosa controlliamo gli errori di allocazione

char* get_string_2(FILE *fp)
{
   char * ptr=(char*)malloc(sizeof(char));
   char * noleak;
   int i=0;
   char ci;
   if(!ptr)return NULL;
    
   ci = fgetc(fp);
    
   while(ci!=10 && ci!=EOF)
   {        
      ptr[i++]=ci;
      noleak = ptr;
      ptr=(char*)realloc(ptr,(i+1)*sizeof(char));        
      if(!ptr)
      {
         free(noleak);
         return NULL;
      }
      ci=fgetc(fp);
   }
   ptr[i]='\0';

   return ptr;
}

terza cosa, realloc è molto pesante, proviamo a ridurre le chiamate a realloc allocando blocchi di N byte
migliorando le prestazioni di un bel po

#define BLOCK_SIZE 80
char* get_string_3(FILE *fp)
{
   size_t AllocSize = BLOCK_SIZE;
   int i=0;
   char ci; 
   char * ptr=(char*)malloc(AllocSize * sizeof(char));
   if(!ptr)return NULL;
   
   ci = fgetc(fp);
   
   while(ci!=10 && ci!=EOF)
   {      
      ptr[i++]=ci;
            
      if(i == AllocSize)
      {
         char * noleak = ptr;
         AllocSize += BLOCK_SIZE;
         ptr=(char*)realloc(ptr,AllocSize*sizeof(char));      
         if(!ptr)
         {
            free(noleak);
            return NULL;
         }
      }
      ci=fgetc(fp);
   }
   ptr[i]='\0';

   return ptr;
}

Quarta e ultima cosa, adesso avendo allocato multipli di N Byte molto probabilmente stiamo sprecando memoria.
Potremmo rilasciare la quantita non utilizzata chiamando ancora realloc.

char* get_string_4(FILE *fp)
{
   size_t AllocSize = BLOCK_SIZE;
   int i=0;
   char ci;
   char * ptr=(char*)malloc(AllocSize * sizeof(char));
   if(!ptr)return NULL;
   
   ci = fgetc(fp);
   
   while(ci!=10 && ci!=EOF)
   {      
      ptr[i++]=ci;      
      
      if(i == AllocSize)
      {
         char * noleak = ptr;
         AllocSize += BLOCK_SIZE;
         ptr=(char*)realloc(ptr,AllocSize*sizeof(char));      
         if(!ptr)
         {
            free(noleak);
            return NULL;
         }
      }
      ci=fgetc(fp);
   }

   ptr[i++]='\0';

   if(i != AllocSize)   
      ptr = (char*)realloc(ptr, i);      

   return ptr;
}

volendo possiamo rendere più elegante il codice

char* get_string_5(FILE *fp)
{
   size_t bufferSize = BLOCK_SIZE;
   char * buffer=(char*)malloc(bufferSize * sizeof(char));   
   size_t usedBuffer = 0;
   char ci;

   if(!buffer)return NULL;
   
   while((ci = fgetc(fp)) != '\n' && ci!=EOF)
   {      
      buffer[usedBuffer++]=ci;
      
      if(usedBuffer == bufferSize)
      {
         char * noleak = buffer;
         bufferSize += BLOCK_SIZE;
         buffer=(char*)realloc(noleak,bufferSize*sizeof(char));      
         if(!buffer)
         {
            free(noleak);
            return NULL;
         }
      }
   }

   buffer[usedBuffer++] = '\0';

   if(usedBuffer != bufferSize)   
      buffer = (char*)realloc(buffer, usedBuffer);         

   return buffer;
}

Infine una variante puramente didattica: se le linee da leggere fossero enormi,
una politica migliore potrebbe essere raddoppiare il buffer ogni volta, in
questo modo per allocare una stringa di 1GB dovrei chiamare realloc 25 volte
invece che 13 milioni di volte (Se BLOCK_SIZE=80)


il problema è che allocare blocchi enormi potrebbe portare a errori di allocazione.

Se ad esempio finora avessi allocato 1GB di memoria per una stringa, e mi serve
1GB + 3 byte, provando ad allocare 2GB potrei avere errore
però in realtà esiste memoria a sufficienza, l'algoritmo quindi prova ad allocare
2GB, se non ci riesce ne alloca 1,5GB , dopo 1,25GB... e continua a provare
fino ad allocare 1GB+1byte.

allo scopo basta sostituire l'algoritmo di riallocazione contenuto nella sezione
if(usedBuffer == bufferSize)

char* get_string_6(FILE *fp)
{
   size_t bufferSize = BLOCK_SIZE;
   char * buffer=(char*)malloc(bufferSize * sizeof(char));   
    size_t usedBuffer = 0;
    char ci;

   if(!buffer)return NULL;
    
    while((ci = fgetc(fp)) != '\n' && ci!=EOF)
    {        
        buffer[usedBuffer++]=ci;
      
      if(usedBuffer == bufferSize)
      {
         size_t SizeIncrease;          
         char * noleak = buffer;
         buffer = NULL;

         for(SizeIncrease = bufferSize; 
            SizeIncrease >= 1 && buffer == NULL; 
            SizeIncrease>>=1)                     
         {
            buffer = (char*) realloc(
                     noleak, 
                     (bufferSize+SizeIncrease)*sizeof(char)
                  );         
         }
         if(!buffer)   {
            free(noleak); 
            return NULL;
         }

         bufferSize+=SizeIncrease;
      }   
    }

    buffer[usedBuffer++] = '\0';

   if(usedBuffer != bufferSize)
   {
      buffer = (char*)realloc(buffer, usedBuffer);      
   }

    return buffer;
}

Infine alcuni test, se compili su microsoft la funzione _msize restituisce la dimensione del blocco. Possiamo fare così verifiche sulle varie varianti per verificare che tutte (tranne la tua) sono corrette. Anche la _1 non è corretta perchè provoca memory leaks nella migliore delle ipotesi o errori nella peggiore. Tutte le altre versioni sono via via più efficienti (fanno meglio la stessa cosa).

Sono sicuro che se poi fai una ricerca su google troverai algoritmi ancora migliori, come ad esempio funzioni in grado di riutilizzare un blocco precedentemente allocato, di lavorare con caratteri unicode etc.

#include <malloc.h> //contiene alcune funzioni non standard microsoft

int main(int argc, char**argv)
{
   char * linea[7];
   char * endptr;
   size_t block_size ;
   size_t slen;
   int i;

   linea[0] = get_string(stdin, NULL);
   linea[1] = get_string_1(stdin);
   linea[2] = get_string_2(stdin);
   linea[3] = get_string_3(stdin);
   linea[4] = get_string_4(stdin);
   linea[5] = get_string_5(stdin);
   linea[6] = get_string_6(stdin);
   

   //la seguente funzione _msize non è standard solo per libreria c microsoft
   
   for(i=0;i<7;++i)
   {
      block_size = _msize(linea[i]);
      slen = strlen(linea[i]);
      endptr = linea[i] + block_size - 1; 
      if(linea[i]==NULL)
         printf("Memoria esaurita per stringa %i", i);
      else 
         printf("Linea %d: %s\nIndirizzo inizio blocco: %d\nIndirizzo fine stringa:%d\n"
               "Ultimo byte nel blocco:%d\nDimensione Blocco:%d\n"
               "Lunghezza stringa:%d\n\n",
            i, linea[i], &linea[i][0], &linea[i][slen], 
            endptr, block_size, slen);
      free(linea[i]);
   }
   
}

[Andrea Simonassi]

dimenticavo un ultima correzione "ci" deve essere int non char in quanto EOF è int e non char.

[ramy89]

Oh cavolo,mi sono accorto che mi potevo risparmiare un mucchio di fatica.
Guarda qua:

char *p;
p=(char*)malloc(sizeof(char));
gets(p);
puts(p);

E mi stampa esattamente la stringa che scrivo in input,anche se ho allocato spazio per un solo carattere.Qualcuno sa spiegarmi perchè si comporta così,e se è corretto usare questo metodo?

[Andrea Simonassi]

no (categorico).