Die Probleme sind, dass Sie einen Zeiger freigeben, der bereits freigegeben worden sein könnte, und Sie nicht wissen, wie viel Speicherplatz verwendet wird haben Sie keinen Zeiger auf den zuletzt zugewiesenen Speicherplatz (im Allgemeinen) Sie können die Erinnerung nicht genau freigeben. In main() haben Sie:
char **images_filenames;
[...]
if ((images_filenames = ((char**) malloc(10 * sizeof (char*)))) == NULL) {
[...]
if (readInputFile(fpin, images_filenames) == -1) {
[...]
deallocate2D(images_filenames, num_lines);
Sie ordnen 10 Zeichenzeiger, und dann das Array übergeben an die readInputFile() Funktion. Innerhalb dieser Funktion gibt es Code, um das Array neu zuzuordnen, aber Sie haben dem Hauptprogramm keine Möglichkeit gegeben, zu wissen, was diese neue Adresse ist. Die Art und Weise, wie Sie das tun, geschieht entweder durch Übergeben eines Zeigers an das, was geändert werden soll, oder Sie geben die Funktion zurück, um den modifizierten Wert zurückzugeben (oder Sie greifen zu schäbigen Praktiken wie globale Variablen anstelle von Parametern) - aber das sollten Sie nicht tun).
So benötigen Sie:
if (readInputFile(fpin, &images_filenames) == -1) {
Und in der readInputFile() Funktion benötigen Sie eine ganze Reihe von Änderungen - die große mit dem Triple-Zeiger Argument zu befassen, und dann eine Vielzahl von Kodierungs Probleme:
int readInputFile(FILE *fp, char ***ppp_files)
{
num_lines = 0;
int s = 10;
char line[MAX_LENGTH];
char **file_images = *ppp_files;
char **final_filenames;
Update: ich habe nicht bemerkt, dass dies einfach num_lines wurde initialisiert, erklärt es nicht. Daher muss num_lines eine globale Variable irgendeiner Art sein ... einige der folgenden Kommentare müssen angepasst werden, um dies zu ermöglichen.
Bis jetzt ist die Änderung (fast) trivial; Wir bekommen einen Zeiger auf ein 'char **', daher das Triple-Pointer-Argument. Um den folgenden Code zu vereinfachen, erstellen Sie eine lokale Kopie des Parameters unter dem alten Namen (file_images) und initialisieren Sie ihn mit dem Wert, auf den das Argument zeigt. Der folgende Code kann dann weiter mit file_images arbeiten; Stellen Sie nur sicher, dass das Argument vor der Rückgabe aktualisiert wird.
Außer ...
Sie gehen davon aus, dass ‚s = 10‘, aber wirklich, sollten Sie die Hauptfunktion Ihnen sagen, wie viele Zeilen zur Verfügung standen. Es hat 10 Reihen zugeteilt, aber es ist nicht ohne eine sorgfältige Überprüfung klar, dass das der Fall war. Sie sollten das main() Programm sagen, wie viele Zeilen zuvor zugewiesen wurden - ein zusätzliches Argument für die Funktion.Sie haben auch das Problem, dass das main() Programm kann die deallocate2D() Funktion nicht sagen, wie viele Zeilen im Array sind, weil es nicht weiß. Es ist nicht klar, wie dein Code kompiliert wird; Sie haben hier eine lokale Variable num_lines, aber es gibt einen Verweis auf eine Variable num_lines in main(), für die es keine Deklaration gibt. Die lokale Variable maskiert jede globale Variable.
while (fgets(line, sizeof line, fp) != NULL) {
if (line[0] != '\n') {
if (num_lines >= s) {
s += 100;
Das Hinzufügen einer großen Anzahl von Zeilen ist eine gute Idee; Es "amortisiert" die Kosten für die Umverteilung.
if ((file_images = (char**) realloc(file_images, s * sizeof (char*))) == NULL)
Es gibt spezifische Probleme mit der Technik, die Sie verwendet haben. reiner Code-Stil: Wenn eine Linie ein if mit einer eingebetteten Zuordnung enthält und es wird zu lang, teilten Sie die Zuordnung aus, bevor die Bedingung:
file_images = (char**) realloc(file_images, s * sizeof (char*));
if (file_images == NULL)
Jetzt gibt es nur eine subtilen Fehler links. Was passiert, wenn realloc() fehlschlägt ...
Das ist richtig, Sie haben den Speicher durchgesickert, weil der Wert in file_images null ist, also gibt es keine Möglichkeit zu veröffentlichen, worauf es hinweist. schreiben Nie:
x = realloc(x, size);
Es verliert Speicher bei einem Fehler! Daher müssen Sie:
char **new_space = realloc(file_images, s * sizeof (char*));
if (new_space == NULL)
{
printf("Error reallocating space for 2d array: %s\n",
strerror(errno));
*ppp_files = file_images;
return -1;
}
}
Als allgemeine Regel gilt, Fehlermeldungen auf stderr gedruckt werden soll; Ich habe das nicht behoben.
Beachten Sie, dass ich vorsichtig den letzten (nicht Null) Wert von file_images in die Variable im Hauptprogramm kopiert. Es könnte auch angebracht sein, dasselbe mit der Größe zu tun (eine andere Schnittstellenänderung) oder eine Struktur zu verwenden, um das Array - Größe und Zeiger auf seine Basis zu kapseln.
if ((file_images[num_lines] = malloc(MAX_LENGTH * sizeof (char))) == NULL)
{
printf("Error allocating space for 2d array: %s\n", strerror(errno));
return -1;
}
Diese Fehlerrückgabe muss *ppp_files = file_images; eingestellt werden.
strncpy(file_images[num_lines], line, MAX_LENGTH);
if (file_images[num_lines] == NULL) {
printf("Strncpy failed: %s\n", strerror(errno));
return -1;
}
Dieser Test ist ungerade; Sie wissen, dass file_images[num_lines] nicht null ist, und strncpy() ändert das nicht. Sie benötigen die Test- und Fehlerbehandlung nicht.
printf("name of file %d is: %s \n", num_lines, file_images[num_lines]);
num_lines++;
}
}
printf("Num_lines: %d\n",num_lines);
OK ...
//realloc to number of lines in the file, to avoid wasting memory
Nice touch. Es lohnt sich kaum; Selbst auf einer 64-Bit-Maschine verschwenden Sie höchstens weniger als 1 KiB. Es schadet jedoch nicht, ordentlich zu sein - gut.
if ((final_filenames = realloc(file_images, num_lines * sizeof (char*))) == NULL) {
printf("Error reallocating space for 2d array: %s\n", strerror(errno));
return -1;
Auch hier müssen Sie *ppp_files = file_images; vor Rückkehr setzen.
} else {
file_images = final_filenames;
Dies hat keinen Einfluss auf den Wert im Programm main(). Es müsste wieder *ppp_files = file_images; sein.
deallocate2D(final_filenames, num_lines);
Festhalten - Sie alle Ihre sorgfältig zugewiesenen Speicherplatz freigeben? Also wirst du es doch nicht benutzen? Die obige Zuweisung kopierte lediglich einen Zeigerwert herum; es hat nicht eine Kopie des Speichers macht ...
}
return 0;
//don't forget to free lines 2d array! (here or at the end of the code)
}
Dieser Kommentar ist falsch - auf erfolgreiche Rückkehr wird der Speicher bereits freigegeben.
Lemme rate - Sie verwenden nicht 'Vim' oder ein anderes 'vi' Derivat zum Bearbeiten. Personen, die in Spalte 1 die öffnende Klammer ihrer Funktionen haben, weil Sie dann mit ']]' oder '[[' vorwärts oder rückwärts durch die Datei zum Anfang der nächsten oder vorherigen Funktion springen können. Es ist ärgerlich, mit Code zu arbeiten, wo das nicht funktioniert.
Nun, das ist eine Startdiagnose ... Hier funktioniert Code mit einer Struktur, um das Array von Dateinamen herum zu übertragen. Ich habe den Körper der readInputFile() Funktion verlassen, die lokale Variablen verwendet, die aus der Struktur heraus kopiert werden, und stellte sicher, dass die Struktur ständig richtig aktualisiert wird.
#include <stdio.h>
#include <stdlib.h>
#include <errno.h>
#include <string.h>
enum { MAX_LENGTH = 512 };
typedef struct FileNameArray
{
size_t nfiles; /* Number of file names allocated and in use */
size_t maxfiles; /* Number of entries allocated in array */
char **files; /* Array of file names */
} FileNameArray;
static void deallocate2D(FileNameArray *names)
{
for (size_t i = 0; i < names->nfiles; i++)
free(names->files[i]);
free(names->files);
names->nfiles = 0;
names->files = 0;
names->maxfiles = 0;
}
static int readInputFile(FILE *fp, FileNameArray *names)
{
int num_lines = names->nfiles;
int max_lines = names->maxfiles;
char **file_names = names->files;
char line[MAX_LENGTH];
char **final_filenames;
while (fgets(line, sizeof line, fp) != NULL)
{
if (line[0] != '\n')
{
/* Remove newline from end of file name */
char *nl = strchr(line, '\n');
if (nl != 0)
*nl = '\0';
if (num_lines >= max_lines)
{
max_lines += 100;
char **space = realloc(file_names, max_lines * sizeof (char*));
if (space == NULL)
{
fprintf(stderr, "Error reallocating space for 2d array: %s\n",
strerror(errno));
return -1;
}
names->maxfiles = max_lines;
names->files = space;
file_names = space;
}
if ((file_names[num_lines] = malloc(strlen(line) + 1)) == NULL)
{
fprintf(stderr, "Error allocating space for 2d array: %s\n",
strerror(errno));
return -1;
}
names->nfiles++;
strcpy(file_names[num_lines], line);
printf("name of file %d is: %s \n", num_lines, file_names[num_lines]);
num_lines++;
}
}
printf("Num_lines: %d\n", num_lines);
//realloc to number of lines in the file, to avoid wasting memory
if ((final_filenames = realloc(file_names, num_lines * sizeof (char*))) == NULL)
{
fprintf(stderr, "Error reallocating space for 2d array: %s\n",
strerror(errno));
return -1;
}
names->maxfiles = num_lines;
names->files = final_filenames;
return 0;
}
int main(int argc, char *argv[])
{
FileNameArray names = { 0, 0, 0 };
//check parameters
if (argc < 4)
{
fprintf(stderr, "Usage: %s input_filename.ppm charWidth charHeight\n",
argv[0]);
return -1;
}
printf("Opening input file [%s]\n", argv[1]);
FILE *fpin = fopen(argv[1], "r");
if (fpin == NULL) {
fprintf(stderr, "Could not open input file %s (%s)\n",
argv[1], strerror(errno));
return -1;
}
if ((names.files = malloc(10 * sizeof (char*))) == NULL)
{
fprintf(stderr, "Error allocating initial space for 2d array: %s\n",
strerror(errno));
return -1;
}
names.maxfiles = 10;
if (readInputFile(fpin, &names) == -1)
{
fprintf(stderr, "Error reading image filenames from input\n");
return -1;
}
fclose(fpin);
printf("###########\n");
deallocate2D(&names);
printf("Done!\n");
return 0;
}
Wow, das ist jetzt eine Analyse! Ich schätze Ihre Bemühungen und werde versuchen, Ihre Vorschläge zu verstehen und anzuwenden, sobald ich einen C-Compiler in die Hände bekomme. Und ja, ich benutze nicht vim oder vi, ich benutze eine IDE wie Netbeans oder Geany.Ich werde Sie auf dem Laufenden halten :) – neverMind
@neverMind: Im Arbeitscode hätte ich den Speicherzuweisungscode in eine Funktion ausgeklammert wie 'int addFileName (FileNameArray * names, const char * line);' und nannte das in 'readInputFile()'. Dann würde es weniger Verwirrung in diesem Eingabecode geben. Und in 'main()' würde es keine Vorabzuweisung geben; die Funktion würde einfach die Zuteilung von Grund auf handhaben. –