Thread: passing pointer to function

Hi all.

Why does the while loop in the main() function keep executing?

Thanks in advance.

Code:

// wordcount.c: count the number of occurrences of every word in a text file
// 4 functions: openfile, countwords, sort, display

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

#define FILENAMELENGTH 30
#define SUCCESS 1
#define FAILURE 0

FILE *fp;

int openfile(char *string);

int main(void)
{
    int result;
    char filename[FILENAMELENGTH];

    printf("Enter file to open: ");
    fgets(filename, FILENAMELENGTH, stdin);
    filename[strlen(filename) + 1] = '\0';


    while((result = openfile(filename)) == FAILURE)
    {
        printf("File could not be opened. Enter file to open: ");
        fgets(filename, FILENAMELENGTH, stdin);
        filename[strlen(filename) + 1] = '\0';
        result = openfile(filename);
    }

    return 0;
}

int openfile(char *string)
{
    if((fopen(string, "r")) == NULL)
        return FAILURE;
    else
    {
        printf("\n%s opened successfully", string);
        return SUCCESS;
    }
}

Oh yes, forgot to assign the file to a pointer. Still stuck in the loop though.

Code:

// wordcount.c: count the number of occurrences of every word in a text file
// 4 functions: openfile, countwords, sort, display

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

#define FILENAMELENGTH 30
#define SUCCESS 1
#define FAILURE 0

FILE *fp;

int openfile(char *string);

int main(void)
{
    int result;
    char filename[FILENAMELENGTH];

    printf("Enter file to open: ");
    fgets(filename, FILENAMELENGTH, stdin);
    filename[strlen(filename) + 1] = '\0';


    while((result = openfile(filename)) == FAILURE)
    {
        printf("File could not be opened. Enter file to open: ");
        fgets(filename, FILENAMELENGTH, stdin);
        filename[strlen(filename) + 1] = '\0';
        result = openfile(filename);
    }

    return 0;
}

int openfile(char *string)
{
    if((fp = fopen(string, "r")) == NULL)
        return FAILURE;
    else
    {
        printf("\n%s opened successfully", string);
        return SUCCESS;
    }
}

Originally Posted by std10093

.Are you sure that the file exsists

Yep, it exists.

Part of the problem is in the lines "filename[strlen(filename) + 1] = '\0';" That is setting a character past the end of the string to zero.

The other part of the problem is that fgets() leaves a '\n' in the input string (in quite a few circumstances) and few filenames contain a '\n' character.

Thanks for the replies. This code works:

Code:

// wordcount.c: count the number of occurrences of every word in a text file
// 4 functions: openfile, countwords, sort, display

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

#define FILENAMELENGTH 30
#define SUCCESS 1
#define FAILURE 0

FILE *fp;

int openfile(char *string);

int main(void)
{
    int result;
    char filename[FILENAMELENGTH];

    printf("Enter file to open: ");
    fgets(filename, FILENAMELENGTH, stdin);
    filename[strlen(filename) - 1] = '\0';

    while((result = openfile(filename)) == FAILURE)
    {
        printf("File could not be opened. Enter file to open: ");
        fgets(filename, FILENAMELENGTH, stdin);
        filename[strlen(filename) - 1] = '\0';
    }

    return 0;
}

int openfile(char *string)
{
    if((fp = fopen(string, "r")) == NULL)
        return FAILURE;
    else
    {
        printf("\n%s opened successfully", string);
        return SUCCESS;
    }
}

I notice that this declaration is at file scope:

Code:

FILE *fp;

It should be local to the openfile function instead.

For this:

Code:

int openfile(char *string);

It would be better to make the parameter a const char* since the string will not be modified, and to use filename as the parameter name instead of string.

Also, this has a small edge case error:

Code:

fgets(filename, FILENAMELENGTH, stdin);
filename[strlen(filename) - 1] = '\0';

Basically, fgets only stores the newline character if there is enough space for it. Hence, it would be better to write:

Code:

if (fgets(filename, FILENAMELENGTH, stdin))
{
    char *p = strchr(filename, '\n');
    if (p)
    {
        *p = '\0';
    }
}