Ошибка при использовании strtok для разделения длинной строки на более короткие строки - PullRequest
0 голосов
/ 07 июня 2018

У меня есть функция, в которой я пытаюсь разбить строку, но каким-то образом она останавливается, когда читается spaces.

input.csv: 18820218,Northern Ireland,England,0,13,Friendly,Belfast,Ireland,FALSE

output.txt: 18820218,Northern,(null),(null),(null),(null),(null),(null),(null)

typedef struct
{
    long int date;
    char *h_team;
    char *a_team;
    int home_score;
    int away_score;
    char *reason;
    char *city;
    char *country;
    char *neutral_field;

}Data;


void open_output(char *string, FILE **output)
{       
    if((*output=fopen(string, "w")) == NULL)
    {
        printf("%s not found\n", string);
            exit(1);
    }
}

void alloc_Data(Data *d, int size)
{
    d->line1 = (char*)malloc(50*sizeof(char)); 
    d->h_team = (char*)malloc(30*sizeof(char)); 
    d->a_team = (char*)malloc(30*sizeof(char)); 
    d->reason = (char*)malloc(30*sizeof(char)); 
    d->city = (char*)malloc(30*sizeof(char)); 
    d->country = (char*)malloc(30*sizeof(char)); 
    d->neutral_field = (char*)malloc(9*sizeof(char)); 
}

void store(Data *d, FILE *output)
{
    char *string = "18820218,Northern Ireland,England,0,13,Friendly,"
                    "Belfast,Ireland,FALSE";
    char *char_date = malloc(10*sizeof(char));
    char *char_hscore = malloc(20*sizeof(char));
    char *char_ascore = malloc(3*sizeof(char));

    char *token;

    token = strtok(string, ",");    
    char_date = token;

    token = strtok(NULL, ",");
    d->h_team = token;  

    token = strtok(NULL, ",");
    d->a_team = token;  

    token = strtok(NULL, ",");
    char_hscore = token;

    token = strtok(NULL, ",");
    char_ascore = token;    

    token = strtok(NULL, ",");
    d->reason = token;  

    token = strtok(NULL, ",");
    d->city = token;    

    token = strtok(NULL, ",");
    d->country = token; 

    token = strtok(NULL, ",");
    d->neutral_field = token;   

    d->date = atoi(char_date);
    d->home_score = atoi(char_hscore);
    d->away_score = atoi(char_ascore);

    fprintf(output, "%li,%s,%s,%d,%d,%s,%s,%s,%s\n", d->date, d->h_team, 
            d->a_team, d->home_score, d->away_score, d->reason, d->city, 
            d->country, d->neutral_field );

    free(string);
    free(char_date);
    free(char_hscore);
    free(char_ascore);
}

int main(int argc, char *argv[])
{
    FILE *output;
    char *string = "saida.txt";

    open_output(string, &output);   

    Data *d;
    d = (Data*)malloc(sizeof(Data)); 
    alloc_Data(d);

    store(d, output);

    free(d);

    return 0;
}

Ответы [ 2 ]

0 голосов
/ 08 июня 2018

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

Цель для динамического размещения ваших структур или данных состоит в том, чтобы (1) обрабатывать большие объемы данных, которые уместятся в стек вашей программы (здесь не проблема), (2) чтобы позволить вамувеличить или уменьшить объем хранилища, которое вы используете, так как ваши потребности в данных изменяются в течение вашей программы (здесь тоже не проблема), или (3) чтобы вы могли адаптировать свои потребности в хранилище на основе данных, используемых в вашей программе,Эта последняя часть, кажется, является тем, что вы пытаетесь сделать, но, выделяя фиксированный размер для ваших символьных массивов, вы полностью теряете преимущество адаптации своего распределения к размеру ваших данных.

Чтобы выделить хранилищедля каждой из строк, содержащихся в ваших данных, вам нужно получить длину каждой строки, а затем выделить length + 1 символов для хранения (+1 для символа , заканчивающегося нулем ).Хотя вы можете использовать malloc, а затем strcpy, чтобы выполнить выделение и скопировать в новый блок памяти, если у вас есть strdup, это может сделать для вас оба в одном вызове функции.

вы сталкиваетесь с проблемой " Где мне хранить данные до того, как я получу длины и распределю копию? " Вы можете справиться с этим несколькими способами.Вы можете объявить путаницу разных переменных и для начала разбить данные на отдельные переменные (несколько грязно), вы можете выделить одну структуру с фиксированными значениями для первоначального хранения значений (хороший вариант, но вызов malloc для 30 или 50 chars не имеет большого смысла, когда подойдет фиксированный массив), или вы можете объявить отдельную временную структуру с фиксированными размерами массива для использования (таким образом, чтобы собрать перемешивание отдельных переменных в структурузатем его можно легко передать в вашу функцию allocate). Рассмотрите каждый из них и используйте тот, который лучше всего работает для вас.

Типы возвращаемых функций не имеют большого смысла, как они есть.Вам нужно выбрать осмысленный тип возвращаемого значения, который позволит функции указывать, успешно она или нет, а затем возвращать значение (или указатель на значение), которое предоставляет полезную информацию для остальной части вашей программы.Оценка успеха / сбоя функции особенно важна для функций, которые выделяют память или управляют вводом или выводом.

В дополнение к выбранному типу возвращаемого значения вам необходимо продумать параметры, передаваемые каждой функции.Вам нужно подумать о том, какие переменные должны быть доступны в вашей функции.Выберите параметр FILE*.Вы никогда не используете файл вне вашей функции store() - так почему вы объявили его в main(), что заставляет вас беспокоиться о возврате открытого потока через указатель - который вы не используете.

Имея это в виду, мы можем немного свести воедино части вашей программы.

Во-первых, не используйте магические числа , разбросанные по всему коду.(например, 9, 10, 20, 30, 50, etc..) Вместо этого

#define MAXN  9     /* if you need constants, define one (or more) */
#define MAXC 30
#define MAXL 50

(или вы можете использовать enum для той же цели)

Для целей примера вы можете использовать структуру, котораявы будете динамически выделять для эффективного хранения ваших данных и временную структуру, чтобы помочь в анализе значений из вашей строки данных.Например:

typedef struct {    /* struct to hold dynamically allocated data */
    long date;      /* sized to exact number of chars required. */
    int home_score,
        away_score;
    char *h_team,
        *a_team,
        *reason,
        *city,
        *country,
        *neutral_field;
} data_t;

typedef struct {    /* temp struct to parse data from line */
    long date;      /* sized to hold largest anticipated data */
    int home_score,
        away_score;
    char h_team[MAXC],
        a_team[MAXC],
        reason[MAXC],
        city[MAXC],
        country[MAXC],
        neutral_field[MAXN];
} data_tmp_t;

Далее, вся цель вашей функции open_output - открыть файл для записи.Он должен вернуть поток открытых файлов в случае успеха или NULL в противном случае, например,

/* pass filename to open, returns open file stream pointer on
 * success, NULL otherwise.
 */
FILE *open_output (const char *string)
{       
    FILE *output = NULL;

    if ((output = fopen (string, "w")) == NULL)
        fprintf (stderr, "file open failed. '%s'.\n", string);

    return output;
}

Ваша функция alloc_data выделяет структуру данных и заполняет ее значения.Он должен вернуть указатель на полностью распределенную и заполненную структуру в случае успеха или NULL в случае неудачи, например

/* pass temporary struct containing data, dynamic struct allocated,
 * each member allocated to hold exact number of chars (+ terminating
 * character). pointer to allocated struct returned on success,
 * NULL otherwise.
 */
data_t *alloc_data (data_tmp_t *tmp)
{
    data_t *d = malloc (sizeof *d); /* allocate structure */

    if (d == NULL)
        return NULL;

    d->date = tmp->date;

    /* allocate each string member with strdup. if not available,
     * simply use malloc (strlen(str) + 1), and then strcpy.
     */
    if ((d->h_team = strdup (tmp->h_team)) == NULL)
        return NULL;
    if ((d->a_team = strdup (tmp->a_team)) == NULL)
        return NULL;

    d->home_score = tmp->home_score;
    d->away_score = tmp->away_score;

    if ((d->reason = strdup (tmp->reason)) == NULL)
        return NULL;
    if ((d->city = strdup (tmp->city)) == NULL)
        return NULL;
    if ((d->country = strdup (tmp->country)) == NULL)
        return NULL;
    if ((d->neutral_field = strdup (tmp->neutral_field)) == NULL)
        return NULL;

    return d;   /* return pointer to allocated struct */
}

Каждый раз, когда вы выделяете несколько значений, которые вложены в структуру (или вложенные структуры), у вас появляется привычка записывать free_data функцию в free память, которую вы выделяете в alloc_data.Намного лучше написать одну бесплатную функцию для правильной обработки сложной структуры, которую вы выделили, по сравнению с разбросом отдельных вызовов free вокруг вашего кода.Нет возврата для проверки при освобождении переменной, поэтому вы можете использовать функцию void здесь:

/* frees each allocated member of d, and then d itself */
void free_data (data_t *d)
{
    free (d->h_team);
    free (d->a_team);
    free (d->reason);
    free (d->city);
    free (d->country);
    free (d->neutral_field);

    free (d);
}

Ваша функция store() - это то место, где выполняется большинство решений и проверок валидации.Цель вашего кода - проанализировать, а затем сохранить string в filename.Это должно заставить вас задуматься о том, какие параметры требуются.Остальная часть обработки файла может быть внутренней до store(), поскольку FILE больше не используется в вызывающей функции.Теперь, в зависимости от того, сколько записей вы выполняете, может иметь смысл объявить и открыть FILE один раз в main(), а затем передать открытый (и проверенный) параметр FILE*, для чего потребуется только одинfopen вызов и окончательный close в main().Для целей здесь все будет обрабатываться в store, поэтому вы можете проверять наличие любой ошибки потока после каждой записи, проверяя возвращаемое значение fclose.

Поскольку вы выделяете и храните структуру, которая может потребоватьсядля дальнейшего использования в вызывающей функции выбор возврата указателя на вызывающего (или NULL при сбое) делает хороший выбор типа возврата для store().Вы можете сделать что-то вроде:

/* parses data in string into separate values and stores data in string
 * to filename (note: use mode "a" to append instead of "w" which
 * truncates). returns pointer to fully-allocated struct on success,
 * NULL otherwise.
 */
data_t *store (const char *string, const char *filename)
{
    data_tmp_t tmp = { .date = 0 };
    data_t *d = NULL;
    FILE *output = open_output (filename);  /* no need to pass in */
                                            /* not used later in main */
    if (output == NULL) {   /* validate file open for writing */
        return NULL;
    }

    /* parse csv values with sscanf - avoids later need to convert values
     * validate all values successfully converted.
     */
    if (sscanf (string, "%ld,%29[^,],%29[^,],%d,%d,%29[^,],%29[^,]," 
                        "%29[^,],%8[^\n]",
                        &tmp.date, tmp.h_team, tmp.a_team, &tmp.home_score,
                        &tmp.away_score, tmp.reason, tmp.city, tmp.country,
                        tmp.neutral_field) != 9) {
        fprintf (stderr, "error: failed to parse string.\n");
        return NULL;
    }

    d = alloc_data (&tmp);  /* allocate d and deep-copy tmp to d */
    if (d == NULL) {        /* validate allocation/copy succeeded */
        perror ("malloc-alloc_data");
        return NULL;
    }

    /* output values to file */
    fprintf (output, "%ld,%s,%s,%d,%d,%s,%s,%s,%s\n", d->date, d->h_team, 
            d->a_team, d->home_score, d->away_score, d->reason, d->city, 
            d->country, d->neutral_field );

    if (fclose (output) == EOF) /* always validate close-after-write */
        perror ("stream error-output");

    return d;   /* return fully allocated/populated struct */
}

Ваш main() может затем обрабатывать не более чем вашу строку, которую нужно проанализировать, имя файла для записи данных и указатель на полностью распределенную структуру, получающуюся в результатеиз разбора, поэтому он доступен для дальнейшего использования.(он также принимает файл для записи в качестве 1-го аргумента программы - или он будет записывать в "saida.txt" по умолчанию, если аргумент не предоставлен, например,

int main (int argc, char *argv[])
{
    char *string = "18820218,Northern Ireland,England,0,13,Friendly,"
                    "Belfast,Ireland,FALSE";
    /* filename set to 1st argument (or "saida.txt" by default) */
    char *filename = argc > 1 ? argv[1] : "saida.txt";
    data_t *d = NULL;

    d = store (string, filename);   /* store string in filename */

    if (d == NULL) {    /* validate struct returned */
        fprintf (stderr, "error: failed to store string.\n");
        return 1;
    }

    /* output struct values as confirmation of what was stored in file */
    printf ("stored: %ld,%s,%s,%d,%d,%s,%s,%s,%s\n", d->date, d->h_team, 
            d->a_team, d->home_score, d->away_score, d->reason, d->city, 
            d->country, d->neutral_field );

    free_data (d);  /* free all memory when done */

    return 0;
}

, хотя и не предписан CСтандартно, «стандартный» стиль кодирования для C исключает использование camelCase или MixedCase имен переменных в пользу всех строчных букв при сохранении прописных имен для использованияс макросами и константами. Это вопрос стиля - так что это полностью зависит от вас, но если вы не будете следовать ему, это может привести к неправильному первому впечатлению в некоторых кругах.

В целом это можно сделатьчто-то вроде следующего:

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

#define MAXN  9     /* if you need constants, define one (or more) */
#define MAXC 30
#define MAXL 50

typedef struct {    /* struct to hold dynamically allocated data */
    long date;      /* sized to exact number of chars required. */
    int home_score,
        away_score;
    char *h_team,
        *a_team,
        *reason,
        *city,
        *country,
        *neutral_field;
} data_t;

typedef struct {    /* temp struct to parse data from line */
    long date;      /* sized to hold largest anticipated data */
    int home_score,
        away_score;
    char h_team[MAXC],
        a_team[MAXC],
        reason[MAXC],
        city[MAXC],
        country[MAXC],
        neutral_field[MAXN];
} data_tmp_t;

/* pass filename to open, returns open file stream pointer on
 * success, NULL otherwise.
 */
FILE *open_output (const char *string)
{       
    FILE *output = NULL;

    if ((output = fopen (string, "w")) == NULL)
        fprintf (stderr, "file open failed. '%s'.\n", string);

    return output;
}

/* pass temporary struct containing data, dynamic struct allocated,
 * each member allocated to hold exact number of chars (+ terminating
 * character). pointer to allocated struct returned on success,
 * NULL otherwise.
 */
data_t *alloc_data (data_tmp_t *tmp)
{
    data_t *d = malloc (sizeof *d); /* allocate structure */

    if (d == NULL)
        return NULL;

    d->date = tmp->date;

    /* allocate each string member with strdup. if not available,
     * simply use malloc (strlen(str) + 1), and then strcpy.
     */
    if ((d->h_team = strdup (tmp->h_team)) == NULL)
        return NULL;
    if ((d->a_team = strdup (tmp->a_team)) == NULL)
        return NULL;

    d->home_score = tmp->home_score;
    d->away_score = tmp->away_score;

    if ((d->reason = strdup (tmp->reason)) == NULL)
        return NULL;
    if ((d->city = strdup (tmp->city)) == NULL)
        return NULL;
    if ((d->country = strdup (tmp->country)) == NULL)
        return NULL;
    if ((d->neutral_field = strdup (tmp->neutral_field)) == NULL)
        return NULL;

    return d;   /* return pointer to allocated struct */
}

/* frees each allocated member of d, and then d itself */
void free_data (data_t *d)
{
    free (d->h_team);
    free (d->a_team);
    free (d->reason);
    free (d->city);
    free (d->country);
    free (d->neutral_field);

    free (d);
}

/* parses data in string into separate values and stores data in string
 * to filename (note: use mode "a" to append instead of "w" which
 * truncates). returns pointer to fully-allocated struct on success,
 * NULL otherwise.
 */
data_t *store (const char *string, const char *filename)
{
    data_tmp_t tmp = { .date = 0 };
    data_t *d = NULL;
    FILE *output = open_output (filename);  /* no need to pass in */
                                            /* not used later in main */
    if (output == NULL) {   /* validate file open for writing */
        return NULL;
    }

    /* parse csv values with sscanf - avoids later need to convert values
     * validate all values successfully converted.
     */
    if (sscanf (string, "%ld,%29[^,],%29[^,],%d,%d,%29[^,],%29[^,]," 
                        "%29[^,],%8[^\n]",
                        &tmp.date, tmp.h_team, tmp.a_team, &tmp.home_score,
                        &tmp.away_score, tmp.reason, tmp.city, tmp.country,
                        tmp.neutral_field) != 9) {
        fprintf (stderr, "error: failed to parse string.\n");
        return NULL;
    }

    d = alloc_data (&tmp);  /* allocate d and deep-copy tmp to d */
    if (d == NULL) {        /* validate allocation/copy succeeded */
        perror ("malloc-alloc_data");
        return NULL;
    }

    /* output values to file */
    fprintf (output, "%ld,%s,%s,%d,%d,%s,%s,%s,%s\n", d->date, d->h_team, 
            d->a_team, d->home_score, d->away_score, d->reason, d->city, 
            d->country, d->neutral_field );

    if (fclose (output) == EOF) /* always validate close-after-write */
        perror ("stream error-output");

    return d;   /* return fully allocated/populated struct */
}

int main (int argc, char *argv[])
{
    char *string = "18820218,Northern Ireland,England,0,13,Friendly,"
                    "Belfast,Ireland,FALSE";
    /* filename set to 1st argument (or "saida.txt" by default) */
    char *filename = argc > 1 ? argv[1] : "saida.txt";
    data_t *d = NULL;

    d = store (string, filename);   /* store string in filename */

    if (d == NULL) {    /* validate struct returned */
        fprintf (stderr, "error: failed to store string.\n");
        return 1;
    }

    /* output struct values as confirmation of what was stored in file */
    printf ("stored: %ld,%s,%s,%d,%d,%s,%s,%s,%s\n", d->date, d->h_team, 
            d->a_team, d->home_score, d->away_score, d->reason, d->city, 
            d->country, d->neutral_field );

    free_data (d);  /* free all memory when done */

    return 0;
}

Пример использования / вывода

$ ./bin/store_teams dat/saida.txt
stored: 18820218,Northern Ireland,England,0,13,Friendly,Belfast,Ireland,FALSE

Проверка выходного файла

$ cat dat/saida.txt
18820218,Northern Ireland,England,0,13,Friendly,Belfast,Ireland,FALSE

Использование памяти / проверка ошибок

Нет необходимости приводить к возврату malloc, в этом нет необходимости. См .: Преобразует ли я результат malloc?

В любом написанном вами коде, который динамически распределяет память, у вас есть 2 отклика* * * * * относительно любого выделенного блока памяти: (1) всегда сохраняет указатель на начальный адрес для блока памяти, поэтому (2) он может быть освобожден , когда онбольше не требуется.

Крайне важно, чтобы вы использовали программу проверки ошибок памяти, чтобы убедиться, что вы не пытаетесь получить доступ к памяти или писать за пределами / за пределами вашего выделенного блока, пытаться прочитать или создать условное выражениеперейти к неинициализированному значению и, наконец, подтвердить, что вы освобождаете всю выделенную память.

Для Linux valgrind - нормальный выбор.Для каждой платформы есть похожие проверки памяти.Все они просты в использовании, просто запустите вашу программу через нее.

$ valgrind ./bin/store_teams dat/saida.txt
==16038== Memcheck, a memory error detector
==16038== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==16038== Using Valgrind-3.12.0 and LibVEX; rerun with -h for copyright info
==16038== Command: ./bin/store_teams dat/saida.txt
==16038==
stored: 18820218,Northern Ireland,England,0,13,Friendly,Belfast,Ireland,FALSE
==16038==
==16038== HEAP SUMMARY:
==16038==     in use at exit: 0 bytes in 0 blocks
==16038==   total heap usage: 8 allocs, 8 frees, 672 bytes allocated
==16038==
==16038== All heap blocks were freed -- no leaks are possible
==16038==
==16038== For counts of detected and suppressed errors, rerun with: -v
==16038== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

Всегда подтверждайте, что вы освободили всю выделенную память и нет ошибок памяти.

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

0 голосов
/ 07 июня 2018

Код, как показано, не будет компилироваться-компилироваться по следующим причинам:

  • Член d->line1 не существует в struct.
  • Функция void alloc_Data(Data *d, int size) имеет два аргумента, но вызов: alloc_Data(d); имеет только 1 аргумент.

Кроме того, поскольку определение для функции open_output(string, &output); не предусмотрено,код не может быть запущен никем, пытающимся помочь.(предположения сделаны вне этой точки)

Помимо этого ...

Это:

    token = strtok(NULL, ",");
    d->h_team = token;  

эффективно изменяет адрес ранееуказатель malloc, приводящий к утечке памяти.(Это связано с тем, что любые последующие вызовы free(d->h_team); будут выполняться с адресом, который никогда не был malloc'ed).

Это:

    token = strtok(NULL, ",");
    strcpy(d->h_team,token);

приводит к назначению содержимого, находящегося вадрес token по адресу, расположенному в d->h_team, что означает, что вы все равно можете позвонить free(d->h_team);, когда закончите с ним.(избегая утечки памяти)

Чтобы обойти сбой, который вы видите, это может помочь:

    char *string = "18820218,Northern Ireland,England,0,13,Friendly,Belfast,Ireland,FALSE";
    char *workingbuf = '\0'

    workingbuf  = strdup(string);
    token = strtok(string, ",");
    ...    

Еще одна мысль, это хорошая идея, чтобы проверить вывод strtok() перед принятием token содержит что-либо:

    token = strtok(NULL, ",");
    if(token)
    {
        d->h_team = token;
        ...  

Редактировать
После внесения изменений, предложенных мной выше, включая добавление open_output,ваш код работал.

...