C++

Forum

 
strcpy and assignment does not work

Alex1993 (15)
I am modifying a program to accept command line parameters and I have two tables called "trades" and "quotes". I want to read these from the command line and assign them to
variable table1 and table2. For some reason, the assign does not work properly. 

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
 
  #include <unistd.h>
  #include <stdlib.h>
  #include <stdio.h>
  #include <string.h>
  #include <errno.h>
  #include "k.h"

  #define KXVER 3

  static const char *optString="t::";


  int main(int argc, char *argv[]){
    int opt=0;
    char table1[30];
    char table2[30];

    printf("%s and %s\n",argv[2],argv[3]);//this one prints ok

    opt=getopt(argc,argv,optString);
    while(opt!=-1){
      switch(opt){
        case 't':
          strcpy(table1,argv[2]);
          strcpy(table2,argv[3]);
          printf("%s and %s\n",table1,table2);//this one prints garbage
        default:
          printf("default case\n");
          return 1;
      }
    opt=getopt(argc,argv,optString);
    }
  printf("table 1 is %s and table 2 is %s\n", table1, table2);

  return 0;
  }
MikeyBoy (5631)
Shouldn't the command-line arguments be argv[1] and argv[2]?
Alex1993 (15)

printf("%s and %s\n",argv[2],argv[3]);

-> this line would have also failed. I think argv[0] is the name of the program, argv[1] is the flag and argv[2] and argv[3] are the tables. However I do not know if this is a valid assign with the getopt function.
Last edited on
MikeyBoy (5631)
What flag? You don't say anything in your OP about passing in a flag as well. Certainly, there's nothing in your code that references argv[1], so whatever you're passing in as the first argument is not being used anywhere.
icy1 (596)
Looking at the linux manual for getopt at http://man7.org/linux/man-pages/man3/getopt.3.html , and also looking at an example https://www.gnu.org/software/libc/manual/html_node/Example-of-Getopt.html .

1. Why not have line 10 with a with a single colon?
Two colons mean an option takes an optional arg; if there is
text in the current argv-element (i.e., in the same word as the
option name itself, for example, "-oarg"), then it is returned in
optarg, otherwise optarg is set to zero. This is a GNU extension.


The double colon appears to invite complications.

2. We can see that the option flag is the output of getopt, "opt" in your case, which is fine. You forgot to make use of the special variable optarg . See the example for reference. I also suspect your input may have spaces, which obviously messes w/ argument parsing altogether.

3. Missing break in your cases. Always break each case unless you intentionally want stuff to fall through.

4. I suggest to just make a second option for the second table, since you're into getopt anyway. 
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
 
 #include <unistd.h>
  #include <stdlib.h>
  #include <stdio.h>
  #include <string.h>
  #include <errno.h>
  #include "k.h"

  #define KXVER 3

  static const char *optString="tu:";

  int main(int argc, char *argv[]){
    int opt;
    char table1[30];  // t option
    char table2[30];  // u option

    // Variables to make sure both flags are specified
    int tflag = 0;
    int uflag = 0;  
    
    while( (opt=getopt(argc,argv,optString)) !=-1){
      switch(opt){
        case 't':
          tflag = 1;
          strcpy(table1, optarg);
          break;

        case 'u':
          uflag = 1;
          strcpy(table2, optarg);
          break;   

        default:
          printf("Usage:  myprogram -t <table1> -u <table2>\n");
          return 0;
      }
    }
  if (!tflag && !uflag)
  {
      printf("Both -t and -u options are required\n");
      return 2;
  }

  printf("table 1 is %s and table 2 is %s\n", table1, table2);

  return 0;
}



See also (from the example) how they check for bad arguments with the special variable optind, which auto-updates with getopt. They have a for loop to cycle through anything else the user might have (erroneously) added and prints out the issue "Non-argument ...".
Last edited on
Alex1993 (15)
So if I expect someone to execute the program -> ./<progName> -t table1 table2 shouldn't I have the flag and two potential options? What if I don't know how many tables the user wants? What if the user wants something like ./<progName> table1 table2 table3 table4 table5 ... tableN where each table has a name that I do not know beforehand, but if passed by user in the correct form I can extract it from a database? Any chance you could point me in the right direction for achieving that? Thanks!
Alex1993 (15)
I managed to find a way to do this on a different site (I do not know if I am allowed to post the link here). 

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
 
    (...)

     int j=0; 

     (...) 

     case 't':
        optind--; //this is the next argument so we decrease it by one

        for (;optind<argc && *argv[optind] != '-'; optind++){ 
        //for loop checks if the optind is less than the number 
        //of arguments and after "&&" we check that the first 
        //character is not a dash which would mark the next flag
          
          tables[j]=(char*) malloc(10); //we allocate memory 
          tables[j]=argv[optind]; //and assign the argument argv[optind]
          
          j++; // increment j by one
        }
        break;    

    (...)


ISSUES:
1. if you have an argument like this "-table" this method will fail because it will interpret it as the next flag
2. the memory allocation is set arbitrary -> not very good

If you can identify any other problems with this code please add it here and thank you all for your contribution
Last edited on
lastchance (6980)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
 
#include <iostream>
#include <string>
#include <cstring>
#include <vector>
using namespace std;

//======================================================================

vector<string> getTables( int argc, char *argv[] )
{
   vector<string> tables;

   // Find any -t flag
   int i = 1;
   while ( i < argc && strcmp( argv[i], "-t" ) ) i++;

   // Now read strings from next item until we exhaust the command line or find another flag
   i++;
   while ( i < argc && argv[i][0] != '-' )
   {
      tables.push_back( argv[i] );
      i++;
   }

   return tables;
}

//======================================================================

int main( int argc, char *argv[] )
{
   vector<string> tables = getTables( argc, argv );

   cout << "\nYour tables are:\n";
   for ( string s : tables ) cout << s << '\n';
}

//====================================================================== 


a.exe -c quotes.txt -t tabLondon tabMadrid tabBerlin -f out.txt 

Your tables are:
tabLondon
tabMadrid
tabBerlin


dhayden (5798)
If you are always passing file names for n tables then you don't need command line options, you need command line arguments. Just pass the names on the command line without the -t option at all. e.g. myprog table1 table2 table3

Are there other possible things you want to pass on the command line? I get the sense that there may be. If that's the case then just pass the table names with separate -t options for each. e.g.: myprog -t table1 -t table2 -t table3

Optind is usually used after the getopt loop to process the command line arguments. Here is an example that allows multiple -t options for multiple tables and also shows how to handle arguments that come after the options.
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
 
#include <unistd.h>
#include <iostream>
#include <vector>
#include <string>

using std::cout;
using std::vector;
using std::string;

void usage(const char *argv0)
{
    cout << "Usage: " << argv0 << " [-t table] [-t table ...] other args\n";
}

int main(int argc, char **argv)
{
    int opt;
    vector<string> tables;
    
    while ((opt = getopt(argc, argv, "t:")) != -1) {
	switch (opt) {
	case 't':
	    tables.push_back(optarg);
	    break;
	default:
	    cout << "Illegal option '" << (char)opt << "'\n";
	    usage(argv[0]);
	    return 1;
	}
    }

    cout << "The tables are";
    for (const string &tab : tables) {
	cout << ' ' << tab;
    }
    cout << '\n';
    cout << "The remaining command line args are:";
    while (optind < argc) {
	cout << ' ' << argv[optind++];
    }
    cout << '\n';
}

$ ./foo this is a test
The tables are
The remaining command line args are: this is a test

dave@zaphod ~/tmp
$ ./foo -t table1 boo
The tables are table1
The remaining command line args are: boo

dave@zaphod ~/tmp
$ ./foo -t table1 -t table2 -t table3
The tables are table1 table2 table3
The remaining command line args are:

dave@zaphod ~/tmp
$ ./foo -t table1 -t table2 -t table3 arg1 arg2 arg3
The tables are table1 table2 table3
The remaining command line args are: arg1 arg2 arg3

Topic archived. No new replies allowed.