Jump to content
Search In
  • More options...
Find results that contain...
Find results in...

Help with C program that prints contents of a file

SgtBot
 Share

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:

image.png.3ccdca9cc47465466205d3bf5efa2a73.png

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.

Link to comment
Share on other sites

Link to post
Share on other sites

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 :P

Don't ask to ask, just ask... please 🤨

sudo chmod -R 000 /*

What is scaling and how does it work? Asus PB287Q unboxing! Console alternatives :D Watch Netflix with Kodi on Arch Linux Sharing folders over the internet using SSH Beginner's Guide To LTT (by iamdarkyoshi)

Sauron'stm Product Scores:

Spoiler

Just a list of my personal scores for some products, in no particular order, with brief comments. I just got the idea to do them so they aren't many for now :)

Don't take these as complete reviews or final truths - they are just my personal impressions on products I may or may not have used, summed up in a couple of sentences and a rough score. All scores take into account the unit's price and time of release, heavily so, therefore don't expect absolute performance to be reflected here.

 

-Lenovo Thinkpad X220 - [8/10]

Spoiler

A durable and reliable machine that is relatively lightweight, has all the hardware it needs to never feel sluggish and has a great IPS matte screen. Downsides are mostly due to its age, most notably the screen resolution of 1366x768 and usb 2.0 ports.

 

-Apple Macbook (2015) - [Garbage -/10]

Spoiler

From my perspective, this product has no redeeming factors given its price and the competition. It is underpowered, overpriced, impractical due to its single port and is made redundant even by Apple's own iPad pro line.

 

-OnePlus X - [7/10]

Spoiler

A good phone for the price. It does everything I (and most people) need without being sluggish and has no particularly bad flaws. The lack of recent software updates and relatively barebones feature kit (most notably the lack of 5GHz wifi, biometric sensors and backlight for the capacitive buttons) prevent it from being exceptional.

 

-Microsoft Surface Book 2 - [Garbage - -/10]

Spoiler

Overpriced and rushed, offers nothing notable compared to the competition, doesn't come with an adequate charger despite the premium price. Worse than the Macbook for not even offering the small plus sides of having macOS. Buy a Razer Blade if you want high performance in a (relatively) light package.

 

-Intel Core i7 2600/k - [9/10]

Spoiler

Quite possibly Intel's best product launch ever. It had all the bleeding edge features of the time, it came with a very significant performance improvement over its predecessor and it had a soldered heatspreader, allowing for efficient cooling and great overclocking. Even the "locked" version could be overclocked through the multiplier within (quite reasonable) limits.

 

-Apple iPad Pro - [5/10]

Spoiler

A pretty good product, sunk by its price (plus the extra cost of the physical keyboard and the pencil). Buy it if you don't mind the Apple tax and are looking for a very light office machine with an excellent digitizer. Particularly good for rich students. Bad for cheap tinkerers like myself.

 

 

Link to comment
Share on other sites

Link to post
Share on other sites

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)

Link to comment
Share on other sites

Link to post
Share on other sites

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 ).

Link to comment
Share on other sites

Link to post
Share on other sites

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 :P

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:

image.png.1d91ff0779ace45b6376c309704de529.png

Link to comment
Share on other sites

Link to post
Share on other sites

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?

Link to comment
Share on other sites

Link to post
Share on other sites

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);
	}
}

 

Link to comment
Share on other sites

Link to post
Share on other sites

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:

image.png.1d91ff0779ace45b6376c309704de529.png

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.

Don't ask to ask, just ask... please 🤨

sudo chmod -R 000 /*

What is scaling and how does it work? Asus PB287Q unboxing! Console alternatives :D Watch Netflix with Kodi on Arch Linux Sharing folders over the internet using SSH Beginner's Guide To LTT (by iamdarkyoshi)

Sauron'stm Product Scores:

Spoiler

Just a list of my personal scores for some products, in no particular order, with brief comments. I just got the idea to do them so they aren't many for now :)

Don't take these as complete reviews or final truths - they are just my personal impressions on products I may or may not have used, summed up in a couple of sentences and a rough score. All scores take into account the unit's price and time of release, heavily so, therefore don't expect absolute performance to be reflected here.

 

-Lenovo Thinkpad X220 - [8/10]

Spoiler

A durable and reliable machine that is relatively lightweight, has all the hardware it needs to never feel sluggish and has a great IPS matte screen. Downsides are mostly due to its age, most notably the screen resolution of 1366x768 and usb 2.0 ports.

 

-Apple Macbook (2015) - [Garbage -/10]

Spoiler

From my perspective, this product has no redeeming factors given its price and the competition. It is underpowered, overpriced, impractical due to its single port and is made redundant even by Apple's own iPad pro line.

 

-OnePlus X - [7/10]

Spoiler

A good phone for the price. It does everything I (and most people) need without being sluggish and has no particularly bad flaws. The lack of recent software updates and relatively barebones feature kit (most notably the lack of 5GHz wifi, biometric sensors and backlight for the capacitive buttons) prevent it from being exceptional.

 

-Microsoft Surface Book 2 - [Garbage - -/10]

Spoiler

Overpriced and rushed, offers nothing notable compared to the competition, doesn't come with an adequate charger despite the premium price. Worse than the Macbook for not even offering the small plus sides of having macOS. Buy a Razer Blade if you want high performance in a (relatively) light package.

 

-Intel Core i7 2600/k - [9/10]

Spoiler

Quite possibly Intel's best product launch ever. It had all the bleeding edge features of the time, it came with a very significant performance improvement over its predecessor and it had a soldered heatspreader, allowing for efficient cooling and great overclocking. Even the "locked" version could be overclocked through the multiplier within (quite reasonable) limits.

 

-Apple iPad Pro - [5/10]

Spoiler

A pretty good product, sunk by its price (plus the extra cost of the physical keyboard and the pencil). Buy it if you don't mind the Apple tax and are looking for a very light office machine with an excellent digitizer. Particularly good for rich students. Bad for cheap tinkerers like myself.

 

 

Link to comment
Share on other sites

Link to post
Share on other sites

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 

Link to comment
Share on other sites

Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now
 Share


×