Структуры и указатели в C - сбой при использовании strcpy

Question

У меня есть задание, которое должно быть написано на C (не на C ++), в котором мне нужно создать некоторые структуры для чтения нескольких текстовых файлов. Я изучил c раньше (2 года назад) - мне гораздо удобнее с Java, просто я не могу использовать это для этого проекта. Я думаю, моя проблема в том, что я не очень хорошо понимаю синтаксис указателя: /. Тем не менее, моя настоящая проблема:

Код, который я написал, вылетает при попытке использовать функцию strcpy:

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

typedef struct{
    char* filename;
    int time;
} JOB;

JOB **jobQueue;
int nJobs;

void trimLine(char* line) {
    for (int i = strlen(line); i >=0; i--) {
        if (line[i] == '\n' || line[i] == '\r') line[i] = '\0';
    }
}

int main(int argc, char* argv[]) {
    if (argc !=2) {
        printf("Error - Usage is: my_project file\n");
        exit(-1);
    }
    FILE *fp;
    fp = fopen(argv[1],"r");
    if (fp==NULL) {
        printf("Error - file %s could not be read.\n",argv[1]);
        exit(-1);
    }
    jobQueue = malloc(3*sizeof(JOB*));
    char filename[BUFSIZ];
    nJobs = 0;
    while (fgets(filename,sizeof(jobfilename),fp)!=NULL) {
        trimLine(filename);    
        JOB* newjob;
        newjob = malloc(sizeof(JOB));
            //** THIS IS WHERE IT SCREWS UP
        strcpy(newjob->filename,filename);

        jobQueue[nJobs++] = newjob;
    }
}

Если я удаляю строку, содержащую strcpy, программа работает нормально (я понимаю, что эта часть ничего не делает, но все же). Однако, когда программа содержит строку strcpy, она прерывается при попытке выполнить задание № 2. Есть идеи почему?

Также: если мне нужно сохранить массив JOB для использования в других функциях, правильно ли я это сделал? JOB ** jobQueue - это массив указателей на JOB, JOB * newjob - указатель на JOB, это будет работать правильно?

Answer 1

Я хотел бы добавить несколько предложений

nJobs = 0;

Глобальные переменные инициализируются с 0, вам не нужно делать это вручную.

while (fgets(filename,sizeof(jobfilename),fp)!=NULL) {

jobfilename не объявленв вашем коде.Я предполагаю, что вы имеете в виду имя файла.

for (int i = strlen(line); i >=0; i--) {
    if (line[i] == '\n' || line[i] == '\r') line[i] = '\0';
}

Вы начинаете с конца \ 0, которое можете пропустить.

Вы объявляете новые переменные везде, где хотите, это хорошая практика (и стандарт C89), чтоповышает читабельность объявления переменных в начале блока кода.

Answer 2

Изменение:

typedef struct{
    char* filename;
    int time;
} JOB;

на:

#include <limits.h>

typedef struct{
    char filename[PATH_MAX];
    int time;
} JOB;

Answer 3

newjob-> имя_файла - это дикий указатель (не установлен на что-либо), вам нужно выделить память, прежде чем вы сможете хранить в нем вещи.

Answer 4

У вас нулевой указатель в newjob->filename:

int nJobsMax=3;
char* filename;
JOB* newjob;
...
jobQueue = malloc(nJobsMax*sizeof(JOB*));
filename=(char*)malloc(BUFSIZ);
while (fgets(filename,BUFSIZ,fp)!=NULL) {       
    trimLine(filename);        
    newjob = (JOB*)malloc(sizeof(JOB));
    newjob->filename = filename;
    filename=(char*)malloc(BUFSIZ);
    jobQueue[nJobs++] = newjob;
    if (nJobs > nJobsMax) 
        //possible buffer overflow need escape

}
free(filename);
fclose(fp);

больше вещей:

void trimLine(char* line) {
   int i = strlen(line)-1;
   do{
        if (line[i] == '\n' || line[i] == '\r') 
            line[i] = '\0';
    }while(!(line[i]>=' ')||i-->=0);                       
} 

на самом деле вам не нужно повторять всю строку
пример: fgetd output => text_text_text \ r \ n \ 0aq
' ' - это значения символьного пространства над этим элементом, символы принтера см. ascii .

fgets () считывает из потока самое большее на один символ меньше размера и сохраняет их в буфере, указанном s. Чтение останавливается после EOF или новой строки. Если читается новая строка, она сохраняется в буфере. Завершающий нулевой байт (aq \ 0aq) сохраняется после последнего символа в буфере.
источник: fgets

strncpy более рекомендуется, чем strcpy, потому что защищает ваш код от переполнения буфера.

Функция strncpy () аналогична, за исключением того, что копируется не более n байтов src. Предупреждение: если среди первых n байтов src нет нулевого байта, строка, помещенная в dest, не будет заканчиваться нулем. Если длина src меньше n, strncpy () записывает дополнительные нулевые байты в dest, чтобы обеспечить запись всего n байтов.
источник: strncpy

другое решение для strcmp:

Функция strdup () возвращает указатель на новую строку, которая является дубликатом строки s. Память для новой строки получается с помощью malloc (3) и может быть освобождена с помощью free (3). Функция strndup () похожа, но копирует не более n байтов. Если s длиннее, чем n, копируется только n байтов, и добавляется завершающий нулевой байт ('\ 0').
источник: strndup

Answer 5

Trasvi, не забывайте, что ваш jobQueue malloc'ed для хранения только 3 экземпляров структуры JOB.Однако ваш цикл while обходится столько раз, сколько вводит пользователь.

Но чтобы ответить на исходный вопрос, просто добавьте его в свой код перед строкой.

newjob->filename = malloc ( strlen( filename) +1 );
//You only need to malloc the amount of characters in the filename + 1,
//which is for the null-terminated char, and you don't need to worry about 
//multiplying by 'sizeof' because a char is one byte on any compiler.

Answer 6

Дополнительные предложения по улучшению вашего кода:

• Вы никогда не free() неправильные указатели.

• Что такое более 3 рабочих мест? Ваш код не справляется с этим. Вы может использовать связанный список вместо массив.

• Вы не набираете fclose() для вашего файла.