18

I'm fairly new to cpp and I am trying to do a project. It says that the code must take in a filename as an argument and will be run by:

./main -i filename

I have written a for-loop that will iterate through the list of arguments to find the "-i" argument so that I can determine the filename. But this line always return false:

argv[i] == "-i"

Below is my code:

#include <string>
#include <iostream>

int main(int argc, char *argv[]) {
    std::string test = argv[0];
    for(int i = 0; i < argc; i++){
        if(argv[i] == "-i"){
            test = argv[i+1];
            break;
        }
    }
    std::cout << test;
    return 1;
}
2
  • I understand you are writing your own parser for learning purposes. Once you know how to do it and you understand the path to achieve it, have a look here. gnu.org/software/libc/manual/html_node/… Oct 2, 2018 at 12:59
  • use function strcmp instead of "argv[i] == "-i"", that is the way you compare two strings in C, while in your code, both sides of "==" decay to pointers.
    – John Z. Li
    Oct 8, 2018 at 9:33

3 Answers 3

23
argv[i] == "-i"

In the line above you compare two pointers: char* and const char*, respectively.

In other words, instead of comparing argv[i] and "-i" two pointers are compared which are pretty much unlikely to point to the same location. As a result, the check doesn't work in your case.

You can fix it in multiple ways, for example wrap "-i" into std::string to make the comparison work properly:

const auto arg = std::string{ "-i" };

for(int i = 0; i < argc; i++){
    if(argv[i] == arg){
        test = argv[i+1];
        break;
    }
}

Starting with C++17 you might also use a std::string_view:

const std::string_view sv{ "-i" };

for(int i = 0; i < argc; i++){
    if(argv[i] == sv){
        test = argv[i+1];
        break;
    }
}

which is a preferable way as it avoids a std::string creation.

12
  • 1
    You really ought to point out the out-of-bounds error if the code is called without a filename following the -i. Probably the best way is to test against argc-1. Oct 2, 2018 at 11:33
  • 3
    It'a also a good idea to start with index 1 as 0 is used for the program name.
    – Thomas
    Oct 2, 2018 at 12:56
  • 5
    It compares two pointers so the result is always false.: not true. Pointer comparison is well defined. Comparison between pointers pointing to the same address is a reasonable operation, which is quite common. The chances of the comparison between a char* literal and a member of argv being equal is pretty much zero, but still technically a legal result. It's a bit nitpicky, but this is SO ¯\_(ツ)_/¯
    – Cyclic3
    Oct 2, 2018 at 16:09
  • 1
    @Cyclic3 it is not nitpicky, it is totally correct :) But for me it is sometimes unclear how detailed the answer should be. Probably, I need to edit the answer and provide a more detailed explanation. Oct 2, 2018 at 17:54
  • 2
    If you’re using C++17, why not use a string literal instead of declaring a completely unnecessary constant? if( argv[i] == "-i"sv) …. Oct 2, 2018 at 21:38
18

You cannot compare pointers to char to string literals (char const*) using ==. Use std::strcmp() (<cstring>) or construct a std::string (<string>) from it to make it comparable to a char* using ==.

4
  • Simplest fix to i out of range is to use i < argc-1 as the limit Oct 2, 2018 at 11:34
  • 1
    or use std::string_view (C++17).
    – Jarod42
    Oct 2, 2018 at 11:55
  • 1
    Note that argv[argc] is defined by the standard to always be valid and null, so it that code is well defined, it just might not be what you expect. Oct 2, 2018 at 17:26
  • @JonathanCallen i'd quote that for truth.
    – Swordfish
    Apr 7, 2019 at 22:12
0

try this:

#include <iostream>
#include <string>
using namespace std;
int main(int argc, char *argv[]) {
    string test;
    for(int i = 0; i < argc; i++){        
        cout << "\n" << argv[i] << endl;
        if((string)argv[i] == "-i"){
            test = argv[i + 1];
            cout << "test= " << test << endl;
            break;
        }
    }
    cout << test << endl;
    system("pause");
    return 0;
}
1
  • 2
    For a good answer, explain what you changed & why it solves the Q. It's unclear what this has, beyond what others already said. I'd also (A) not advise C-style casts & (B) take a serious look at where you index to argv[i + 1]. What's going to happen if the -i is the final argument & you haven't bothered checking whether there is one after it you can index to? Nothing good - just undefined behaviour (i.e. a crash if OP is lucky, and a poisoned program). Edit: OK, not quite, but still Oct 2, 2018 at 19:38

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.