Error in fgets function

Asked

Viewed 910 times

2

The program gives Segmentation fault (core dump) when executing the fgets function. Any idea?

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

main (int argc, char **argv)
{
    FILE *fp=NULL;
    char *f_name;

    switch(argc)
        {
        case(1):fgets(f_name,100,stdin);break;  
        case(2):f_name=argv[1];break;   
        default:printf("\nErro de syntax %s", argv[0]); exit(0);
        }

fputs(f_name, stdout);

fp=fopen(f_name, "r");

if (fp==NULL)
printf("\nNão abriu.\n");
else 
printf("\nAbriu\n");

}
  • Take a look at [tour]. You can accept an answer if it solved your problem. You can vote on every post on the site as well. Did any help you more? You need something to be improved?

3 answers

6

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

int main (int argc, char **argv) {
    FILE *fp = NULL;
    char f_name[100];

    switch(argc) {
        case(1): fgets(f_name, 100, stdin); break;  
        case(2): strncpy(f_name, argv[1], 100); break;   
        default: printf("\nErro de syntax %s", argv[0]); exit(0);
    }

    fputs(f_name, stdout);
    fp = fopen(f_name, "r");
    if (fp == NULL)
        printf("\nNão abriu.\n");
    else 
        printf("\nAbriu\n");
}

I put in the Github for future reference.

This will allocate the memory needed to store the content read in stdin with the fgets(). In this case the declaration as array ensures the prior allocation of memory in the stack.

It is also possible to keep the use with pointer and giving a malloc() after the declaration and before the fgets(), but I think for you it will be easier to use the form of array, you’ve used before.

There are other errors in the code that have now been fixed in the above example. The main thing is that in the second case was copying the wrong content, using the function strncpy() you copy byte by byte the contents of the first argument passed to the program.

Not directly related to the problem, but to better understand about memory management, see What are and where are the "stack" and "heap"?.

2

You have a problem here between the use of arrays and pointers.

char *f_name;

Here declared a pointer with no defined value. When does:

fgets(f_name, 100, stdin);

The fgets() expects to receive in the first argument a pointer pointing to some memory with at least 100 bytes. The first is the pointer to your buffer and the second is the size of it. When you passed an uninitialized pointer, you are doing the fgets() write somewhere any unknown memory, causing your crash.

The solution, as Maniero said, is to create a array with 100 chars right at the beginning, so:

char f_name[100];

When does:

fgets(f_name, 100, stdin);

You are now passing a pointer to the first element of array (write down f_name in that case is equivalent to &f_name[0]). You effectively passed a pointer to a buffer of 100 bytes, everything will work. Except...

f_name = argv[1];

Here f_name is a array, and set the value of arrays is illegal. It doesn’t even make much sense. What you really want is to copy the data that is on argv[1] to the array. For that use the strncpy, thus:

strncpy(f_name, argv[1], 100);

The function will make the copy of up 100 bytes from source to destination.

  • As I commented in the other answer, strncpy is not as safe as it seems, as it does not necessarily terminate the resulting string with ' 0'. Solutions? You can use snprintf, strncat, or strncpy followed by f_name[99] = '\0'.

2

As I was already writing comments a little complex in the other answers, I decided to add mine.

I am using 2 separate variables, one to store characters and one to point to the desired array (f_buffer or argv[1]).

The code below is not tested, and must be missing at least one thing: remove the ' n' that fgets leaves at the end of the array, just before ' 0' (the ' n' represents the Enter that the user typed). Other functions remove ' n', but fgets do not, allowing you to differentiate a truncated input from a complete input. Doing this treatment is like "exercise for the reader" (how I hated it when I read it in a book, hahahaha...). I changed the printf’s at the end to indicate better what the program is doing.

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

#define BUFFER_SIZE 100

int main (int argc, char **argv)
{
    FILE *fp = NULL;
    char *f_name, f_buffer[BUFFER_SIZE];

    switch(argc) {
        case 1:
            fgets(f_buffer, BUFFER_SIZE, stdin);
            /* A FAZER: remover o '\n' do fim do do nome. */
            f_name = f_buffer;
            break;
        case 2:
            f_name = argv[1];
            break;
        default:
            printf("\nErro de syntax %s", argv[0]);
            exit(0);
    }

    fputs(f_name, stdout);

    fp = fopen(f_name, "r");

    if (fp == NULL)
        printf("\nNão abriu [[%s]].\n", f_name);
    else 
        printf("\nAbriu [[%s]]\n", f_name);
}

Browser other questions tagged

You are not signed in. Login or sign up in order to post.