Argument checking

Question

#include <iostream>
#include <fstream>
#include <string>
using namespace std;

int main (int argc, char* argv[])
{
    string STRING;
    ifstream infile;    
    STRING = argv[1];
    infile.open(argv[1]);   
    if (infile.fail())// covers a miss spelling of a fail name
    {
        cout << "ERROR. Did you make a mistake in the Spelling of the File\n";
        return 1;
    }
    else
    {
        while(!infile.eof())
        {
            getline(infile,STRING); // Get the line
            cout<<STRING + "\n"; // Prints out File line
        }   
        infile.close(); 
        return 0; 
    }
}

I have got this program working fine apart from one problem

if the user only runs the program with no file name (what I believe to be called arguments) e.g ./displayfile then I get a Segmentation fault

How would I amend my code so that the program would exit with an error message along the lines of "Add a file name"

My first thought is something along the lines of

if (!argc=2)
{
    cout << "ERROR. Enter a file name";
    return 1;
}

ADDED: just in case this matters am compiling using g++ displayfile.cpp -o displayfile

Well yes, the usual practice is to at least check the `argc` value. — jv42, Nov 18 '11 at 13:17
By the way, you don't need to call `infile.close`, and neither `infile.open` nor `infile.fail`, and calling `infile.eof` like you do it now is even wrong. — Christian Rau, Nov 18 '11 at 13:27
Please pick up a good book and familiarize yourself with the basics of C++. Anytime you feel like writing `if (!argc=2)`, you know you still have a long way to go. — Kerrek SB, Nov 18 '11 at 13:44

Cyclonecode · Accepted Answer · 2013-06-25T22:52:05.763

7

#include <iostream>
#include <fstream>
#include <string>
using namespace std;

int main (int argc, char* argv[]) {
   if(argc != 2) {
      cout << "You need to supply one argument to this program.";
      return -1;
   }

   string STRING;
   ifstream infile;    
   STRING = argv[1];
   infile.open(argv[1]);   
   if (infile.fail())// covers a miss spelling of a fail name {
      cout << "ERROR. Did you make a mistake in the Spelling of the File\n";
      return 1;
   }
   else {
      while(!infile.eof()) {
         getline(infile,STRING); // Get the line
         cout<<STRING + "\n"; // Prints out File line
      }   
      infile.close(); 
      return 0; 
   }
}

edited Jun 25 '13 at 22:52

answered Nov 18 '11 at 13:17

Cyclonecode

29,115
11
72
93

i have added this code to my program but i am still getting a segmentation fault – Dan1676 Nov 18 '11 at 13:29
2

Nice explanation by the way. ;-) – netcoder Nov 18 '11 at 13:45
Well, we could always use something like: You have made a mistake and therefore I wont execute=) – Cyclonecode Nov 18 '11 at 13:48
1

I know that it was in the original code, but please don't encourage the use of `ios::eof()` as a loop condition. It almost always results in buggy code. Prefer: `while(std::getline(infile, STRING)) { std::cout << STRING << "\n"; }` – Robᵩ Nov 18 '11 at 15:18
I agree but like you said I just took the original and modified it to solve the actual problem. – Cyclonecode Nov 18 '11 at 16:05
@dalloliogm - You are thinking about the argument with the full path to the executable? – Cyclonecode Jun 24 '13 at 09:41
@KristerAndersson Yes, this code returns the error message "You need to supply two arguments to this program", while actually it checks that there is only one command line argument apart from the path to executable. – dalloliogm Jun 25 '13 at 12:44

score 2 · Answer 2 · answered Nov 19 '11 at 11:37

Besides the obvious check for argc != 2 I couldn't help fixing some of the worse code and the obvious error:

#include <iostream>
#include <fstream>
#include <string>
using namespace std;

int main (int argc, char* argv[])
{
    if (argc != 2)
    {
        cout << "ERROR. Invalid number of arguments\n";
        return 1;
    }

    ifstream infile(argv[1]);  
    if (!infile)        // covers a miss spelling of a fail name
    {
        cout << "ERROR. Did you make a mistake in the Spelling of the File\n";
        return 1;
    }

    string STRING;
    while(getline(infile, STRING))
        cout << STRING << '\n';      // Prints out file line
    return 0;
}

You don't need to call ifstream::open, just use the constructor, likewise don't you need to declare STRING so early and neither to initialize it to the file name, since you don't use it. Don't forget, this is not C, you don't need a whole mess of declarations at the beginning of each function.

Second, checking for the flags of a stream is often a bad idea, just check for !infile to find any errors. But the real error is in checking for infile.eof in the while condition, since it only gets set once getline has tryed to read over the end of the file, so you would actually print one (probably empty) line too much. Just check for getline's return value to find any errors or the end of file.

And don't add the newline onto the string when outputting, just put it out after the string. And last but not least, no need for infile.close, since the destructor calls it anyway.

score 0 · Answer 3 · answered Nov 18 '11 at 13:17

0

change STRING = argv[1]; to if (argv[1] != null) STRING = argv[1]; not sure though, so you'll have to test it out first.

answered Nov 18 '11 at 13:17

Kevin

2,739
33
57

Won't work, since in this case `argv[1]` is already an error. – Christian Rau Nov 18 '11 at 13:21
1

It won't cause an error per say, but you will be accessing the array out of its bounds, and this is undefined behavior. – netcoder Nov 18 '11 at 13:25
@user1053730: To prevent this, you need to know the size of the array. You have that information in `argc` (see the other answers). – netcoder Nov 18 '11 at 13:41
2

@netcoder - "*[per se](http://en.wikipedia.org/wiki/List_of_Latin_phrases_(P)#per_se)*", not "*per say*". – Robᵩ Nov 18 '11 at 15:16

Argument checking

3 Answers3