2014-02-13 5 views
1

В конце метода все мои тестовые printfs печатают те же результаты. Последняя строка файла. Но текущий printf в цикле while работает правильно. По какой-то причине мои узлы имеют одинаковые результаты. Как я могу это исправить? Это моя структура единица:Связанный список дает те же результаты C

struct unit 
{ 
    struct unit * next; 
    char *name; 
}; 

Это моя функция для связанного списка добавления строк по одному в связанном списке:

void readFile(char fileName[], struct unit * units) 
{ 
    FILE * fp; 
    char *line = NULL; 
    int length = 1000; 
    fp = fopen(fileName, "r"); 
    int counter = 0; 
    int strLength = 0; 
    struct unit * current; 

    units = (struct units*)malloc(sizeof(struct unit)); 
    current = units; 

    while (getline(&line, &length, fp) != -1) 
    { 
     strLength = strlen(&line); 
     current->name = (char*)malloc(sizeof(char)* strLength); 
     current->next = (struct units*)malloc (sizeof(struct unit)); 
     strcpy(&current->name, &line); 
     printf("\nCurrent: %s",current->name); 
     current = current->next; 
     counter++; 
    } 
    printf("\nTest %s", units->name); 
    printf("\nTest %s", units->next->name); 
    printf("\nTest %s", units->next->next->name); 
    printf("\nTest %s", units->next->next->next->name); 
} 
+1

Я бы рекомендовал получить доску или лист бумаги и вытащить ее, чтобы следовать вашей логике. Это поможет вам найти ошибку. – jia103

+0

[Пожалуйста, не делайте результат malloc в C.] (http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc) –

+0

Где определяется getline? – Shmoopy

ответ

0

Ваш код имеет несколько плохих практик, а также несколько ошибок , Перед входом в цикл вам не нужно предварительно распределять узел. Вы можете просто выделить по мере необходимости. Существует несколько способов обеспечить, чтобы вновь выделенный узел был добавлен в конец списка, используя метод, называемый прямой цепью. Я займусь этим через минуту. Следующий список составлен в произвольном порядке, но я по крайней мере пытался работать он сверху вниз


Основные: Ваша readFile() функция должна возвращение глава списка выделяемом. Если вы хотите присоединиться к этому в какой-то другой список после этого, не стесняйтесь, но функция должна начинаться с этим:

struct unit* readFile(const char fileName[]) 

Minor: Следует также отметить, что мы не изменяя имя файла, так что нет разум передать его как изменчивый, поэтому он не const const.


Major: Проверьте файл открыт для работы успеха , прежде чем использовать его:

fp = fopen(fileName, "r"); 
if (fp == NULL) 
{ 
    perror("Failed to open file."); 
    return NULL; 
} 

Основные: Используйте правильно набран переменная для API вызова своего решения. Функция getline(), нестандартное расширение, Прототип:

ssize_t getline(char ** , size_t * , FILE *) 

Он возвращает ssize_t (с «подписью» размером-типа) и занимает size_t* для второго параметра. Вы передаете адрес переменной int, length, в качестве второго параметра. Это не гарантия того, что два являются совместимыми типами.Исправьте это, объявив length подходящим типом; size_t

size_t length = 0; 

Minor: Та же проблема происходит с типом возвращаемого значения strlen(), который также size_t, но это будет неважно в данный момент, как вы скоро увидите.


Основные: Использование getline() кроме второго параметра упоминается перед почти правильно. Начальный ввод в первом цикле - это адрес указателя NULL и 0-значный length. С каждой итерацией, если буфер, уже выделенный в предыдущем цикле, является достаточно большим, он используется повторно. К сожалению, чтение более короткой строки, затем более длинное, , затем короче, и затем дольше вводит дополнительные ассигнования, которые не нужны. На самом деле, вы можете полностью отказаться от своей логики malloc() и просто использовать getline() для выделения своего буфера, поскольку документировано использование malloc() совместимого распределения. Поэтому, используя существующую логику (которую мы будем перейти на последок):

while (getline(&line, &length, fp) != -1) 
{ 
    // note: added to throw out empty lines 
    if (length > 0) 
    { 
     // note: added to null out trailing newline. see the 
     // documentation of getline() for more info. 
     if (line[length-1] == '\n') 
      line[length-1] = 0; 
    } 

    if (line[0] != 0) 
    { 
     // other code here 
     current->name = line; 
    } 
    else 
    { // not using this. release it. 
     free(line); 
    } 

    // reset line and length for next iteration 
    line = NULL; 
    length = 0; 
} 

Major: Ваш оригинальный алгоритм никогда не free() d буфер строки, как только вы сделали с ним, вводя тем самым утечку памяти одноразовую , Используя вышеперечисленную альтернативу, вам не нужно беспокоиться об этом.


Альтернативные: Наконец, петля список популяция может быть более надежным, применяя все обсуждается до сих пор, и добавление к нему технологию, называемую вперед-цепочки. В этом методе используется указатель-указатель pp, который всегда содержит адрес указателя, который получит следующее распределение узла. Если список изначально пуст (и он есть), он содержит адрес указателя head. С каждым новым добавленным узлом pp присваивается адрес последнего члена next. Когда цикл завершен (даже если я не добавил ни одного узла), мы закончим настройкой *pp = NULL, чтобы завершить список.

Это конечная кодовая база для readFile. Я надеюсь, что вам это будет полезно:

struct unit* readFile(char fileName[]) 
{ 
    FILE * fp; 
    char *line = NULL; 
    size_t length = 0; 

    // used for populating the list 
    struct unit *head = NULL; 
    struct unit **pp = &head; 

    // open file 
    fp = fopen(fileName, "r"); 
    if (fp == NULL) 
    { 
     perror("Failed to open file"); 
     return NULL; 
    } 

    while (getline(&line, &length, fp) != -1) 
    { 
     // note: added to throw out empty lines 
     if (length > 0) 
     { 
      // note: added to null out trailing newline. see the 
      // documentation of getline() for more info. 
      if (line[length-1] == '\n') 
       line[length-1] = 0; 
     } 

     if (line[0] != 0) 
     { 
      // allocate new node 
      *pp = malloc(sizeof(**pp)); 
      if (*pp != NULL) 
      { 
       (*pp)->name = line; 
       pp = &(*pp)->next; 
      } 
      else 
      { // could not allocate a new node. uh oh. 
       perror("Failed to allocate new node"); 
       free(line); 
       break; 
      } 
     } 
     else 
     { // not using this. release it. 
      free(line); 
     } 

     // reset line and length for next iteration 
     line = NULL; 
     length = 0; 
    } 
    *pp = NULL; 

    return head; 
} 
1

Почему вы переходящего в &line в strlen и strcpy? Если я правильно помню, вы должны просто передать line и current->name в эти функции. (Я не знаю о getline, хотя, возможно, это нормально, как есть.)

1

Это сработало для меня (построено и запущено с файлом с несколькими строками. Мне пришлось изменить функцию getline для моего компилятора: также изменено несколько «единиц» за «единицу», которая является именем структуры Кроме того, линия для буферизации статический защищена с максимальной длиной 255 символов):.

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

struct unit{ 
    struct unit * next; 
    char *name; 
}; 

void readFile(char fileName[], struct unit * units){ 
    FILE * fp; 
    char line[255]; 
    int length = 1000; 
    fp = fopen(fileName, "r"); 
    int counter = 0; 
    int strLength = 0; 
    struct unit * current; 

    units = (struct unit*)malloc(sizeof(struct unit)); 
    current = units; 

    while (fgets (line, sizeof line, fp) != NULL) /* read a line */ 
    { 
     strLength = strlen(line); 
     current->name = (char*)malloc(sizeof(char)* strLength); 
     current->next = (struct unit*)malloc (sizeof(struct unit)); 
     strcpy(current->name, line); 
     printf("\nCurrent: %s",current->name); 
     current = current->next; 
     counter++; 
    } 
    fclose (fp); 

    printf("\nTest %s", units->name); 
    printf("\nTest %s", units->next->name); 
    printf("\nTest %s", units->next->next->name); 
    printf("\nTest %s", units->next->next->next->name); 
} 

int main(){ 
    readFile("filename.txt", NULL); 
} 
+0

, он работал благодаря.Ничего себе, я не видел, что написал «единицы» вместо «единицы». Проблема, вероятно, в этом. –

+0

Значит, кто-нибудь знает, что такое ошибка? – jia103

+0

@ jia103 С одной стороны, длина буфера имен не включает в себя пространство для завершающего нульчара, поэтому каждый 'strcpy()' работает один раз-слот, передавая конечное выделенное пространство. Странно, этот пример делает то же самое, но по какой-то причине был принят, когда он явно вызывает неопределенное поведение. Во-вторых, этот ответ также делает ту же ошибку, что и код OP, а именно последний узел в списке всегда не используется (включая пустой список). Короче говоря, я понятия не имею, почему это было принято гораздо меньше, устраняя проблему. – WhozCraig

Смежные вопросы