Linux cat command in C

Question

I wrote my own implementation of the linux cat command in C for my computer laboratory class. We were asked to replicate its functionality with no options passed as arguments or with just the -b, -n and -s options.

I'm posting the code here because, although it works fine, i feel a little bit insecure about. Firstly because i think i "over-coded" it a bit, specially on the option handling part, and also, because i'm still not very confortable working with pointers and so i am not very sure if my approach does not generate memory leaks or malfunctions.

Here it is:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <ctype.h>
#define BUF_SIZE 1024
int readStdin(int index, int bflag, int nflag){
 char buffer[BUF_SIZE];
 while(fgets(buffer, BUF_SIZE, stdin)){ //reads from the standard input and prints the input
 if (nflag){
 printf(" %d %s", index, buffer);
 index++;
 }
 if (bflag){
 if (*buffer == '\n'){
 printf ("%s", buffer);
 }
 else{
 printf (" %d %s", index, buffer);
 index++;
 }
 }
 else {
 printf("%s", buffer);
 }
 }
 return index; //returns the incremented index to perpetuate its use
}
int readFile(char* filename, FILE* fp, int index, int bflag, int nflag){
 char ch;
 char s[BUF_SIZE];
 if (fp==NULL){ //in case the file doesn't exist
 printf("%s: No such file or directory\n", filename);
 exit(1);
 }
 if (bflag){
 while ((fgets(s,BUF_SIZE,fp))){
 if (strcmp(s,"\n") == 0){
 printf (" %s", s);
 }
 else {
 printf (" %d %s", index, s);
 index++;
 }
 }
 fclose(fp);
 }
 if (nflag){
 while ((fgets(s,BUF_SIZE,fp))){
 printf (" %d %s", index, s);
 index++;
 }
 fclose(fp);
 }
 else{
 while ((ch=fgetc(fp)) != EOF){ //printing loop
 putchar(ch);
 }
 fclose(fp);
 }
 return index;
}
void readArgs(int argc, char* argv[]){
 FILE* fp;
 int index = 1; //line index. to be used in case -b or -n is passed as an argument
 int option; //option passed as argument
 int bflag = 0; //-b option deactivated by default
 int nflag = 0; //-n option deactivated by default
 opterr = 0; //deactivates getopt's default error messages
 //checks if there are options passed as argument and updates their flags
 while ((option = getopt(argc, argv, "bn")) != -1){
 switch (option){
 case 'b':
 bflag = 1;
 break;
 case 'n':
 nflag = 1;
 break;
 case '?': //in case there was some problem
 exit(1);
 }
 }
 if (bflag == 1 && nflag == 1){ //if -b and -n are passed as argument, b overrides n
 nflag = 0;
 }
 for (int i=optind; i<argc; i++){
 if (*argv[i] == '-'){ //in case of '-' in argv[i], reads from stdin and prints
 index = readStdin(index, bflag, nflag);
 clearerr(stdin);
 }
 else { //prints the contents of the file in *argv[i]
 fp = fopen(argv[i], "r");
 index = readFile(argv[i], fp, index, bflag, nflag);
 }
 }
}
int main(int argc, char* argv[]){
 if (argc<2){ //if there are no arguments
 readStdin(1,0,0);
 return 0;
 }
 readArgs(argc, argv); //otherwise
 return 0;
}

I didn't implement the -s option because i cant think of a way to handle it at the same time as the other options. That's why i think that the way i am handling the options part is a bit inefficient.

Any suggestions?

• esote: Are you allowed to use Linux system calls like read(2) and write(2), or are you forced to use the C standard functions?
• userUSeruser: @esote there are no objections to that. I might take a look into it

Answer 1

Use Standard Constants When You Can

The stdio.h header file contains a definition of BUFSIZ. This is a good constant to use for input and output buffers.

In the stdlib.h header file are 2 constants that are useful when calling exit or returning from main, these are EXIT_FAILURE and EXIT_SUCCESS. These constants are universal, even if the underlying operating system expects different numbers for success or failure this constants will provide the correct value.

DRY Code
There is a programming principle called the Don't Repeat Yourself principle. The principle is this, if you find yourself repeating code, it might be better to create a function or sub-program and call that function where the code is repeated. This reduces the amount of code in a module and makes it easier to read. What is more beneficial than the readability is the fact that you only have to write and debug that code once. Another benefit of puting repeating code into a function is that if the code needs to be changed, there is only one place to change it. .

Code that repeats a lot in this program:

 printf(" %d %s", index, buffer);
 index++;

While this is only one line of code, making a function out of it reduces the possibility of errors such as missing spaces or too many spaces.

All of the output could be in their own functions:

void formattedLineOut(int *index, char buffer[])
{
 printf(" %d %s", *index, buffer);
 *index++;
}

The function formattedLineOut is receiving a reference to index rather than the value. This way the function doesn't have to have a return value for index.

void bprint(int *index, char buffer[])
{
 if (strcmp(s, "\n") == 0) {
 printf(" %s", s);
 }
 else {
 formattedLineOut(index, s);
 }
}
void outputLine(int *index, char buffer[], int nflag, int bflag)
{
 if (nflag)
 {
 formattedLineOut(index, buffer);
 }
 else if (bflag)
 {
 bprint(index, buffer);
 }
 else
 {
 printf("%s", buffer);
 }
}
int readFile(char* filename, FILE* fp, int index, int bflag, int nflag) {
 char ch;
 char s[BUF_SIZE];
 if (fp == NULL) { //in case the file doesn't exist
 printf("%s: No such file or directory\n", filename);
 exit(1);
 }
 while ((fgets(s, BUF_SIZE, fp))) {
 outputLine(&index, s, bflag, nflag)
 }
 return index;
}

Match fclose() With fopen()

The function readFile has multiple calls to fclose(). This can lead to bugs, it might be better to call fclose() after readFile returns in the calling function. This will also allow readFile to handle stdin, since stdin is a file pointer.

• userUSeruser: i'm having a problem with the index pointer. it is not updating its value. how can i approach this?
• pacmaninbw: @GuilhermeLima try (*index)++
• userUSeruser: oh right, i forgot the parentheses. another thing, it is no longer reading from stdin if i pass just an option (or more) and nothing else after that. (for example, ./my_cat -n). how can i approach this? i'm really scratching my head
• pacmaninbw: If you still need to hand in your homework I can't help you, it is beyond the scope of this website. Hand in what you had working. I also can't answer without seeing the code. Since it is broken you might want to post it on stackoverflow.com.