[C]concatenare stringhe allocate dinamincamente

[rossonero922]

questa è la funzione di concatenazione

char *concatena(char *stringaa)
{
char *s;
int risp,i=0;   
printf("quante stringhe vuoi concatenare\n");
scanf("%d",&risp);
fflush(stdin);
s=malloc((sizeof(stringaa)*risp)+1);
if(risp!=0)
printf("inserisci la stringa\n");
gets(stringaa);
strcpy(s,stringaa);
do
{
printf("inserisci la stringa\n");
gets(stringaa);
strcat(s,stringaa);
i++;
}while(i<risp-1);

return s;

}

questo è il main

char stringa[100];
printf("%s\n",concatena(stringa));
 system("PAUSE");

per quel poco che l ho testato sembra che fa il suo dovere..

[oregon]

E quindi? La domanda qual è?

[rossonero922]

E quindi? La domanda qual è?

se scritta cosi la funzione di concatenazione va bene o no

[MItaly]

Al di là del fatto che il codice non è indentato ( ), c'è più o meno un errore per riga...

scanf("%d",&risp);

Qui non stai controllando il valore restituito da scanf, che ti dice se è riuscita ad acquisire l'intero o meno.

fflush(stdin);

Non è portabile; per quanto riguarda lo standard, è undefined behavior, e gcc infatti lo ignora completamente.

s=malloc((sizeof(stringaa)*risp)+1);

sizeof non restituisce la lunghezza "logica" della stringa, ma semplicemente le sue dimensioni. Dato che stringaa è un puntatore a char, restituirà sempre e comunque la stessa dimensione (4 su una macchina a 32 bit, 8 su una a 64 bit), e non le dimensioni del buffer passato alla funzione, che, se ti servono, devono essere passate separatamente. In ogni caso, non è chiaro perché tu voglia allocare risp per le dimensioni di stringaa, visto che comunque non sai quanto sarà lunga la stringa acquisita.

gets(stringaa);

gets è deprecata, dato che acquisisce tranquillamente stringhe di lunghezza qualunque, indipendentemente dalla lunghezza del buffer. Già qui rischi un buffer overflow.

strcat(s,stringaa);

Stai concatenando stringhe di lunghezza sconosciuta ad un buffer di dimensioni limitate, è la ricetta per un buffer overflow.
Inoltre, ogni volta che usi strcat questa si deve ripercorrere tutta la stringa per capire dove finisce e quindi effettuare la copia. Dato che concateni tante volte, le performance di questo sistema sono pessime.

printf("%s\n",concatena(stringa));

concatena ti restituisce una stringa allocata con malloc, ma la passi direttamente alla printf, senza salvare da qualche parte il puntatore, che va così perduto, e quindi non la puoi deallocare con free. È vero che qui il programma termina subito, ma in ogni caso hai un memory leak.

system("PAUSE");

Non è portabile; usa una getchar() piuttosto.

Per poterti suggerire un approccio alternativo, dovresti dire quali dovrebbero essere le specifiche di questa funzione, ovvero, di base cosa deve fare? A cosa dovrebbe servire il buffer passato? Intendi gestire la concatenazione di stringhe di lunghezza arbitraria?

Per inciso, ho tagliato un attimo il titolo, che dovrebbe essere giusto una descrizione sintetica del problema.

[rossonero922]

Al di là del fatto che il codice non è indentato ( ), c'è più o meno un errore per riga...

scanf("%d",&risp);

Qui non stai controllando il valore restituito da scanf, che ti dice se è riuscita ad acquisire l'intero o meno.

fflush(stdin);

Non è portabile; per quanto riguarda lo standard, è undefined behavior, e gcc infatti lo ignora completamente.

s=malloc((sizeof(stringaa)*risp)+1);

sizeof non restituisce la lunghezza "logica" della stringa, ma semplicemente le sue dimensioni. Dato che stringaa è un puntatore a char, restituirà sempre e comunque la stessa dimensione (4 su una macchina a 32 bit, 8 su una a 64 bit), e non le dimensioni del buffer passato alla funzione, che, se ti servono, devono essere passate separatamente. In ogni caso, non è chiaro perché tu voglia allocare risp per le dimensioni di stringaa, visto che comunque non sai quanto sarà lunga la stringa acquisita.

gets(stringaa);

gets è deprecata, dato che acquisisce tranquillamente stringhe di lunghezza qualunque, indipendentemente dalla lunghezza del buffer. Già qui rischi un buffer overflow.

strcat(s,stringaa);

Stai concatenando stringhe di lunghezza sconosciuta ad un buffer di dimensioni limitate, è la ricetta per un buffer overflow.
Inoltre, ogni volta che usi strcat questa si deve ripercorrere tutta la stringa per capire dove finisce e quindi effettuare la copia. Dato che concateni tante volte, le performance di questo sistema sono pessime.

printf("%s\n",concatena(stringa));

concatena ti restituisce una stringa allocata con malloc, ma la passi direttamente alla printf, senza salvare da qualche parte il puntatore, che va così perduto, e quindi non la puoi deallocare con free. È vero che qui il programma termina subito, ma in ogni caso hai un memory leak.

system("PAUSE");

Non è portabile; usa una getchar() piuttosto.

Per poterti suggerire un approccio alternativo, dovresti dire quali dovrebbero essere le specifiche di questa funzione, ovvero, di base cosa deve fare? A cosa dovrebbe servire il buffer passato? Intendi gestire la concatenazione di stringhe di lunghezza arbitraria?

Per inciso, ho tagliato un attimo il titolo, che dovrebbe essere giusto una descrizione sintetica del problema.

prima di tutto grazie per avermi dato una risposta decente........l esercizio dice di concatenare una serie di stringhe ottenute in ingresso....essendo un esercizio così fatto tanto per passare il tempo non è che mi sono messo a fare tutti i controlli...comunque grazie per gli accorgimenti...

s=malloc((sizeof(stringaa)*risp)+1);

il prodotto per risp lo dovevo togliere perche avevo in mente un altra cosa..ma poi so andato avati e me lo so dimenticato.....invece per quanto riguarda il sizeof va bene se lo cambio con strlen()??? poi se metto un controllo sulla gets potrei continuare ad usarla?
per quanto riguarda strcat() come potrei risolvere invece?
per il fatto che il risultato concatena lo passo direttamente direttamente alla printf senza salvare il puntatore mi sono semplicemente attenuto all esercizio...è chiaro che se dovevo farci qualcosa il puntatore me lo salvavo...

[Scara95]

prima di tutto grazie per avermi dato una risposta decente........l esercizio dice di concatenare una serie di stringhe ottenute in ingresso....essendo un esercizio così fatto tanto per passare il tempo non è che mi sono messo a fare tutti i controlli...comunque grazie per gli accorgimenti...

Se è un esercizio e lo risolvi male, a cosa serve farlo?

s=malloc((sizeof(stringaa)*risp)+1);

il prodotto per risp lo dovevo togliere perche avevo in mente un altra cosa..ma poi so andato avati e me lo so dimenticato.....invece per quanto riguarda il sizeof va bene se lo cambio con strlen()???

strlen(stringa)*sizeof(char)

casomai, ma non ne vedo l'utilità dato che quel parametro in input contiene solo spazzatura e non sai se c'è un '\0' e non sai quanto spazio verrà allocato e...

poi se metto un controllo sulla gets potrei continuare ad usarla?

Non puoi mettere un controllo sulla gets, usa fgets invece.

per quanto riguarda strcat() come potrei risolvere invece?

Concatena a mano/scrivi una funzione che conserva uno stato/ aggiungi un indicatore della lunghezza all'inizio in modo da sapere sempre dove sta la fine

per il fatto che il risultato concatena lo passo direttamente direttamente alla printf senza salvare il puntatore mi sono semplicemente attenuto all esercizio...è chiaro che se dovevo farci qualcosa il puntatore me lo salvavo...

MItaly stava proprio puntualizzando che tu DEVI farci qualcosa con quel puntatore, ossia liberare la memoria allocata.

[rossonero922]

Se è un esercizio e lo risolvi male, a cosa serve farlo?

strlen(stringa)*sizeof(char)

casomai, ma non ne vedo l'utilità dato che quel parametro in input contiene solo spazzatura e non sai se c'è un '\0' e non sai quanto spazio verrà allocato e...

Non puoi mettere un controllo sulla gets, usa fgets invece.

Concatena a mano/scrivi una funzione che conserva uno stato/ aggiungi un indicatore della lunghezza all'inizio in modo da sapere sempre dove sta la fine

MItaly stava proprio puntualizzando che tu DEVI farci qualcosa con quel puntatore, ossia liberare la memoria allocata.

ok grazie