0

What I am trying to do is print out the contents of a file line by line. I run the program in terminal by doing: ./test testText.txt. When I do this, random characters are printed out but not what is in the file. The text file is located in the same folder as the makefile. What's wrong?

#include <stdio.h>

FILE *fp;
int main(int argc, char *argv[])
{


char line[15];
fp = fopen(*argv, "r");

while((fgets(line, 15, fp)) != NULL)
    {
        printf(line);
        printf("\n");
    }


}
mh234
  • 37
  • 6

4 Answers4

1

When I do this, random characters are printed out but not what is in the file

These characters are not random, and in fact they are coming from a file. It's not the file that you are trying to read, though - it's the executable file which you are running.

*argv represents the name of the executable; add this line to see what's in *argv:

printf("%s\n", *argv);

The actual command line arguments start at argv[1], so you need

fp = fopen(argv[1], "r");
Sergey Kalinichenko
  • 714,442
  • 84
  • 1,110
  • 1,523
  • @mh234 You are welcome! Since your program works now, consider accepting an answer by clicking the grey check mark next to it. This would let other site visitors know that you are no longer actively looking for an improved answer to this question, and earns you a brand-new badge on Stack Overflow. – Sergey Kalinichenko May 29 '14 at 21:32
1

The first argument passed on the command line is at argv[1], while *argv refers to argv[0]. argv[0] contains the filename of the executable - you are printing out the content of the executable.

The following code prints out the entire argv[] array, then reads your file and prints it.

#include <stdio.h>

int main( int argc, char *argv[] )
{
    for( int i = 0; i < argc; i++ )
    {
        printf( "argv[%d] : %s\n", i, argv[i] ) ; 
    }

    if( argc >= 2 )
    {
        FILE* fp = fopen( argv[1], "r" ) ;
        if( fp != NULL )
        {
            char line[15];

            while( fgets( line, sizeof(line), fp ) != NULL )
            {
                printf( "%s", line ) ;
            }
        }
    }

    return 0 ;
}

Note that fgets() will read an entire line including the , so there is no need to print '\n', especially because with only 15 characters, your line buffer may well not contain an entire line. Note also the tighter localisation of variables - your code needlessly made fp global.

Other refinements are the safe use of the array size rather than literal 15, and the use of a literal constant string for the format specifier. You should avoid passing a variable string for the printf() format string - if your input itself contains format specifiers, printf() will try to read data from arguments that do not exist with undefined results.

Clifford
  • 88,407
  • 13
  • 85
  • 165
1

Q: What's wrong?

A humble critique:

#include <stdio.h>

FILE *fp; // Perhaps this should be declared inside main?

int main(int argc, char *argv[])
   {
   char line[15]; // Are the file lines all 14 characters or less?  (seems small)

   fp = fopen(*argv, "r"); // Opening the binary executable file (argv[0])? Intereting.
   // Should check here to ensure that fopen() succeeded.      

   while((fgets(line, 15, fp)) != NULL)

OK... well, remember that this isn't a text file.. it's an executable (due to *argv). This will read some wacky (but not random) characters from the executable.

      {
      printf(line);  // Bad practice.  Should be: printf("%s", line);

Ok... now print the wacky characters?

      printf("\n");  // Redundant. The '\n' characters will be supplied in 'line'.
      }

   // fclose() call missing.

   // Integer return value for main() is missing.
   }

Here is (perhaps) what was actually intended:

#include <stdio.h>
#include <errno.h>

int main(int argc, char *argv[])
   {
   int rCode = 0;
   FILE *fp = NULL;
   char line[255+1];

   if(argc != 2)
     {
     printf("Usage: %s {filepath}\n", *argv);
     goto CLEANUP;
     }

   errno=0;
   fp = fopen(argv[1], "r");
   if(NULL == fp)
      {
      rCode=errno;
      fprintf(stderr, "fopen() failed. errno:%d\n", rCode);
      goto CLEANUP;
      }

   while(fgets(line, sizeof(line), fp))  /* --As per 'chux' comment */
      printf("%s", line);

 CLEANUP:

   if(fp)
      fclose(fp);

   return(rCode);
   }

Or, if the intent is truly to print the content of the executable, perhaps this:

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

int main(int argc, char *argv[])
   {
   int rCode = 0;
   FILE *fp = NULL;
   off_t offset = 0;

   errno=0;
   fp = fopen(*argv, "r");
   if(NULL == fp)
      {
      rCode=errno;
      fprintf(stderr, "fopen() failed. errno:%d\n", rCode);
      goto CLEANUP;
      }

   for(;;)
     {
     char line[16];
     size_t bytesRead;
     int index;
     char ascii[16+1];

     memset(ascii, 0, sizeof(ascii));
     bytesRead = fread(line, 1, sizeof(line), fp);
     if(0==bytesRead)
        break;

     printf("   %08zX | ", offset);
     for(index=0; index < bytesRead; ++index)
        {
        printf("%02hhX%c", line[index], 7==index ? '-' : ' ');
        ascii[index] = isprint(line[index]) ? line[index] : '.';
        }

     printf("%*s   %s\n", (16 -index) * 3, "", ascii);
     offset += bytesRead;
     }

   if(errno)
      {
      rCode=errno;
      fprintf(stderr, "fgets() failed.  errno:%d\n", errno);
      }

CLEANUP:

   if(fp)
      fclose(fp);

   return(rCode);
   }
Mahonri Moriancumer
  • 5,993
  • 2
  • 18
  • 28
  • Note: `fgets()` is not specified to affect `errno`. Why the `errno=0`? Does not seem to hurt to set to zero or test at the end though. Would not a check of `if (ferror(fp)) ...` after `fgets()` loop make more sense? – chux - Reinstate Monica May 29 '14 at 22:02
  • @chux, you are correct. Do you know what an 'idiot light` is? ...it's the `check engine` light that replaced the oil pressure and water temp gages in cars? **I hate functions that have idiot lights!** For me, I don't appreciate just 'EOF on end of file or error.'. I want to know what happened a little more granularity. (sorry... its my thing). I fixed the answer. – Mahonri Moriancumer May 29 '14 at 22:22
  • Interesting "print the content of the executable" code. Minor suggestions: open with `"rb"` and using `isprint((unsigned char) line[index])` or make `line` an `unsigned char` array. `isprint()` may have trouble with negative values passed to it. Anyways nice answer. – chux - Reinstate Monica May 29 '14 at 22:47
  • To distinguish "end of file or error" after receiving an `EOF` from `fgets()`, simply call `ferror()` and `feof()`. Normally, 1 of those will be true. For exception see http://stackoverflow.com/questions/23388620/is-fgets-returning-null-with-a-short-bufffer-compliant – chux - Reinstate Monica May 29 '14 at 22:50
  • @chux, of course you are correct about ferror(). Back to my __idiot light__ analogy; to determine the cause of the light requires plugging your car into a laptop (analogous to ferror()) to read a code. Then you can look-up that code in the car's documentation to find out what the problem is. Why didn't they put a small display in the car to just show the code! (sorry again for the rant). – Mahonri Moriancumer May 29 '14 at 23:10
0

your file name found at index 1 of argv.

if (argc <= 1) {
    printf("no file was given\n");
    exit(-1);
}

// open file from argv[1]
// ...
eitank
  • 330
  • 1
  • 5