CODEWITHSUNDEEP
carrays
Paulo H
Paulo H

Reputation: 77

Coping a Line from a File to Array (C)

Given a file with information sorted by line, I'm asked to write a program to store each line in a string. 

This is the first line. 
This is the second line. 
This is the third line.
This is the fourth line.

My code is as follows:

#include<stdio.h>
#include<stdlib.h>
#include<string.h>
#define MAX 70
#define NLine 4
int main()
{
    FILE *arq;
    char line[MAX];
    char *eof;
    char name[20];
    char *nline[NLine];
    int i;
    printf("\nGive me the name of the file: ");
    fgets(name, 20 , stdin);
    arq = fopen(name, "r");
    while( ( eof = fgets(line, MAX, arq ) )!= NULL )
        {
            nline[i] = strdup(eof);
            i++;
        }
    printf("\n");
    for(i = 0; i < NLine; i++)
        printf("%s", nline[i]);
    for(i = 0; i < NLine; i++)
        free( nline[i] );
    fclose(arq);
    return 0;
}

My output is a segmentation fault; but when I substitute arq = fopen(name, "r"); for arq = fopen("test.txt", "r"); the program runs perfectly. I execute gdb and it has returned 

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff7a7ea2b in fgets () from /lib64/libc.so.6

Is it wrong to call fopen with an array of strings as a parameter, or did I do something wrong in the program?

Also I would like to know how the part 

    while( ( eof = fgets(line, MAX, arq ) )!= NULL )
        {
            nline[i] = strdup(eof);
            i++;
        }

works. I just saw the code in this question and managed it to work; but I did not understand how the function read another line instead of keeping reading the same line, for there is no apparent change in how the eof is read from the file.

Upvotes: 0

Views: 96

Answers (2)

paddy
paddy

Reputation: 63471

You did not initialise your pointers in the nline array. You need to set each one to NULL. Otherwise, when you come along and free them later, you will have undefined behaviour.

for( i = 0; i < NLine; i++ ) nline[i] = NULL;

Alternatively, you can save the number of lines that you read in and only free that many values, instead of all NLine of them. You should probably do this, because you need to do the same when you print the values out. You'll want to have a count. I've used line_count here.

for( i = 0; i < line_count; i++ ) free(nline[i]);

You have potential buffer overflow when you read the lines in. You need to make sure your array indices stay in range:

line_count = 0;
while( line_count < NLine && fgets(line, MAX, arq) )
    nline[i++] = strdup(line);

And yet another thing, as pointed out by the user Myforwik, fgets leaves the line separator character in the string, and you must remove it yourself when you read in the file name:

if( fgets(name, 20 , stdin) ) {
    /* Remove line terminator(s) if any: iscntrl() from ctype.h is okay */
    for( i = strlen(name)-1; i >= 0 && iscntrl(p[i]); i-- ) p[i] = '\0';
} else {
    name[0] = '\0';
}

Upvotes: 1

Myforwik
Myforwik

Reputation: 3588

fgets will include a carrage return in name, so you are trying to open a file that has a carriage return at the end of the name, which will fail. Since you never check if the program successfully opened the file, the next fgets will be undefined behaviour.

For a start you need to add: if (!arq) { printf("could not open file"); return; } after you attempt fopen.

The other part is simple, it is assigns eof to the output of fgets, then if compares eof to null.

Also you have massive other problems. char *eof; does not allocate room for a string. it allocates a pointer only. you probably want something like char *eof; eof = malloc(1024); You also have this problem with your array of lines. You need to allocate memory to each line. You can't just copy the string to a point - those points need to point to some memory first.

Upvotes: 1

Related Questions