Help with C program that prints contents of a file

[SgtBot]

Hi,

I am trying to write a program that will print out the contents of a file. I also wanted to try making the program take an argument in the command line. Here is my code:

#include <stdio.h>

int readFile(char *infile)
{
    int num;
    FILE *fptr;

    fptr = open("/home/students/hansong5/crypter/%c", infile, "r");
    if(fptr == NULL)
    {
        printf("Error opening file");
    }

    fscanf(fptr, "%d", &num);
    fclose(fptr);

    return num;
}

int main (int argc, char *argv[]) {
    printf(readFile(argv[1]));

    return 0;
}

I am having a number of problems. The first being that this code does not compile without warnings or errors, as shown below:

For some reason, when I try to read the file using fopen, I get the error: "too many arguments". I am trying to specify the path name by putting the %c at the end of the path and specifying that it should use the first argument in the command as the path name.

Then, I get a warning about making a pointer from an integer when using printf. Should I define a variable outside of the printf statement with the function instead of passing the function inside of the printf statement?

The second problem is that when I try to run the code with warnings I get a segmentation fault. I believe that is just because I am misusing the pointers in my code but I'm not sure how to go about fixing this.

 

Any help would be greatly appreciated!

 

 

 

Disclaimer:

While in my filepath you can see "student" as one of the directories, this particular project is not a homework assignment. I program mostly on a redhat server that the university provides us. I am trying to learn about reading and writing files and writing C programs with arguments on my own time to work on a crypter. I always specify whether or not the code I am sharing is part of a homework assignment because helping with homework may go against some people's ethics. I have post history with homework assignments I've worked on as proof of this.

[Sauron]

First of all, that's not how you call printf. You need to specify a format string. In your case, the correct call would be:

printf("%d\n", readFile(argv[1]));

you can find this information in the man page for printf

 

secondly, if you want to declare the file as a pointer, you should use fopen and not open.

 

The rest looks fine to me, but the compiler will be the judge of that

[mariushm]

In your code, if the file can not be opened you're printing the error message, but you're still trying to read and return a number afterwards.

You should have an else { } and put the stuff after there.

A lot of programmers prefer to put the "meat" of the if at the start and something smaller (error handling) at the bottom in the else. So it would probably be more readable, easier to understand to write it like this   if the file handle is NOT null then do lots of things  ELSE  say error message

 

here's a rewrite attempt of that function, note I'm writing it in this post editor, not checking and compiling it, may contain errors

 

int readFile(char *infile)
{
    int num;
    FILE *fptr;

    fptr = open("/home/students/hansong5/crypter/%c", infile, "r");
    if (fptr !== NULL) {
    	fscanf(fptr, "%d", &num);
    	fclose(fptr);
    } else {
        printf("Error opening file");
        // you HAVE to return something, best to initialize num with a value i guess
        num = -1;  
    }
    return num;
}

 

I'd suggest getting used NOT having accolades  { } by themselves on single lines ... you may think it makes your code more readable but it really doesn't ... makes keeping track of elses and where each if ends harder (if you have an if within an if or something like that)

[Unimportant]

8 hours ago, SgtBot said:

<snip>

When you compile with the flag "-Wimplicit-function-declaration" you should get:

Quote

warning: implicit declaration of function ‘open’;

Which means the compiler has not seen a prototype for the function "open" yet (put simply: it does not exist) and has implicitly declared one as:

int open();

Which is where your first error comes from:

Quote

warning: assignment makes pointer from integer without a cast [-Wint-conversion]

The implicitly declared open function returns a "int" which you then try to assign to a "FILE" pointer.

 

You should use "fopen" instead. The reason "fopen" does not work is because you're giving it wrong parameters.

 

Your second error:

Quote

warning: passing argument 1 of ‘printf’ makes pointer from integer without a cast

Is due to the fact that you're passing the result of the "readfile" function, which is of type "int" to "printf", which expects a "char" pointer, pointing to the string to print.

 

Here's an example of what I think you're trying to do:

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

int 
ReadFile(const char *infile)
{
	int num;
	FILE *fptr;

	fptr = fopen(infile, "r");
	if (!fptr)
	{
		printf("Error opening file!\n");
		exit(1);	//abort program with nonzero return code, indicating error.
	}

	/* fscanf returns the number of items successfully extracted, which should be 1...
	   because we ask to read a single integer. */
	if (fscanf(fptr, "%d", &num) < 1)
	{
		printf("Failed to read a valid number!\n");
		exit(2);
	}

	return num;
}

int
main(int argc, char *argv[])
{
	if (argc != 2)
	{
		printf("Incorrect amount of command line parameters!\n");
		return 3;
	}

	printf("Number read from file: %d\n", ReadFile(argv[1]));

	return 0;
}

( Note that this example is simply a modification of your code to make it work. The whole program structure is questionable tough - Opening and reading the file should be handled by separate functions, removing the need for "exit", for example ).

[SgtBot]

12 hours ago, Sauron said:

First of all, that's not how you call printf. You need to specify a format string. In your case, the correct call would be:

printf("%d\n", readFile(argv[1]));

you can find this information in the man page for printf

 

secondly, if you want to declare the file as a pointer, you should use fopen and not open.

 

The rest looks fine to me, but the compiler will be the judge of that

That is actually pretty embarrassing for me... I guess programming at 4 am isn't always the best idea. Anyways, I fixed those issues in my code now, and my new problem is a singular error stating that I am using too many arguments to the fopen function as shown below:

[SgtBot]

5 hours ago, Unimportant said:

When you compile with the flag "-Wimplicit-function-declaration" you should get:

Which means the compiler has not seen a prototype for the function "open" yet (put simply: it does not exist) and has implicitly declared one as:

int open();

Which is where your first error comes from:

The implicitly declared open function returns a "int" which you then try to assign to a "FILE" pointer.

 

You should use "fopen" instead. The reason "fopen" does not work is because you're giving it wrong parameters.

 

Your second error:

Is due to the fact that you're passing the result of the "readfile" function, which is of type "int" to "printf", which expects a "char" pointer, pointing to the string to print.

 

Here's an example of what I think you're trying to do:

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

int 
ReadFile(const char *infile)
{
	int num;
	FILE *fptr;

	fptr = fopen(infile, "r");
	if (!fptr)
	{
		printf("Error opening file!\n");
		exit(1);	//abort program with nonzero return code, indicating error.
	}

	/* fscanf returns the number of items successfully extracted, which should be 1...
	   because we ask to read a single integer. */
	if (fscanf(fptr, "%d", &num) < 1)
	{
		printf("Failed to read a valid number!\n");
		exit(2);
	}

	return num;
}

int
main(int argc, char *argv[])
{
	if (argc != 2)
	{
		printf("Incorrect amount of command line parameters!\n");
		return 3;
	}

	printf("Number read from file: %d\n", ReadFile(argv[1]));

	return 0;
}

( Note that this example is simply a modification of your code to make it work. The whole program structure is questionable tough - Opening and reading the file should be handled by separate functions, removing the need for "exit", for example ).

That code definitely works well, so another question I ask is if I wanted to read the entire contents of a file and print them out, say in this case if it had numbers and letters. Would that be possible with this code or would I need to instead read the binary of the file and convert it after I read the file?

[Unimportant]

5 hours ago, SgtBot said:

That code definitely works well, so another question I ask is if I wanted to read the entire contents of a file and print them out, say in this case if it had numbers and letters. Would that be possible with this code or would I need to instead read the binary of the file and convert it after I read the file? 

The simplest way is to read the file character by character and print each character out until EOF is reached:

#include <stdio.h>

int
main(int argc, char *argv[])
{
	if (argc != 2)
	{
		printf("Incorrect amount of command line parameters!\n");
		return 1;
	}

	FILE* fptr = fopen(argv[1], "r");
	if (!fptr)
	{
		printf("Error opening file!\n");
		return 2;
	}

	while (1)
	{
		int ch = getc(fptr);
		if (ch == EOF)
		{
			return 0;
		}

		putchar(ch);
	}
}

 

[Sauron]

8 hours ago, SgtBot said:

That is actually pretty embarrassing for me... I guess programming at 4 am isn't always the best idea. Anyways, I fixed those issues in my code now, and my new problem is a singular error stating that I am using too many arguments to the fopen function as shown below:

That would be because fopen only needs the file path and the opening mode (read the man page!).

 

I see @Unimportant already corrected you there though.

[Hrillo666]

Hi sorry to barge in - I did this exercise back at uni, only for the teacher to come back and say that "cat and less uses memory mapped files, so you were all wrong...". So google mmap and munmap if you want extra credit