Assigning values from argv[]

This is a discussion on Assigning values from argv[] within the C Programming forums, part of the General Programming Boards category; Hi, I'm currently working on a program that accepts user input on the command line then outputs the mean, median, ...

  1. #1
    Registered User
    Join Date
    May 2009
    Location
    Oregon, USA
    Posts
    9

    Assigning values from argv[]

    Hi,

    I'm currently working on a program that accepts user input on the command line then outputs the mean, median,
    mode, and variance of the data. I'm not concerned with how to do the calculations for the output, however when I'm encountering a problem when I try to use atoi() to assign the strings entered to a single dimensional array to be passed to the function to put all the numbers entered into order. At this point the only function I've finished is sort() which sorts them into order.

    Example of ideal i/o:

    Input: ./mmm 15 18 13 17 18
    Output: 15 18 13 17 18

    Example of actual i/o:

    Input: ./mmm 15 18 13 17 18
    Output: 0 0 0 0 0 0

    Thanks for any help you can give. And I apologize if it's not the best written program, I'm still a beginnning programmer and am bound to make a lot of mistakes.

    - London Fog

    Code:
    int main( int argc, char *argv[])
    {
    	int args_main[MAX]; /* Array used to hold user input. (MAX is defined as 128 in common.h) */
    	int *args_main_p = &args_main[0]; /* Pointer used for allocation of memory */
    	int control_1  = 0, control_2 = 1; /* Variable used for program control (ie. For loops etc) */
    
    	args_main_p = (int*) malloc(MAX*sizeof(int)); /* Allocating memory for user input */
    
    	if( args_main_p != NULL ) /* If memory allocates successfully */
    	{
    
    		for( control_1 = 0 ; control_1 < MAX ; control_1++) // Initializing array
    		{
    			args_main[control_1] = 0;
    
    		}
    	
    		if( argc < 2 ) /* If no arguments are entered */
    		{
    			printf("\n Enter --help for help. (Duh)\n\n");
    			exit(1);
    		}
    	
    		if( argv[1][0] == '-' && argv[1][1] == '-' && argv[1][2] == 'h' && argv[1][3] == 'e' && argv[1][4] =='l' )
    		{
    			printf("\n Enter at least between 2 and 128 integers seperated by spaces.");
    			printf("\n The program will output the mean, median, mode");
    			printf("\n and variance of those integers.");
    			printf("\n Ex Input: 15 18 13 17 18 \n");
    			printf("\n Ex Output: \n mean: 16.2 \n median: 17 \n mode: 18 \n variance: 0 \n");
    			exit(1);
    		}
    
    		do
    		{
    			args_main[control_1] = atoi(argv[control_2]);
    			control_1++; control_2++;
    		} while( atoi(argv[control_2]) >= 1 && atoi(argv[control_2]) <= 9 ); 	
    
    		control_1 = 0;
    			
    		do						// Checking to make sure user
    		{					       // values were passed to args_main
    			printf("%d  ", args_main[control_1]); // correctly
    			control_1++;
    		} while( control_1 < argc-1 );	
    	
    		printf("\n Calling function sort() ");
    	
    		sort( args_main, argc ); /* Call function sort */
    		
    		return 0;
    	}
    	else /* If memory allocation fails */
    	{
    		printf("\n Memory allocation err. Killing program.\n");
    		exit(-1);
    	}
    }
    Last edited by London Fog; 12-16-2009 at 12:51 PM.

  2. #2
    Registered User
    Join Date
    Oct 2006
    Location
    Canada
    Posts
    1,243
    Code:
    		for( control_1 = 0 ; control_1 < MAX ; control_1++) // Initializing array
    		{
    			args_main[control_1] = 0;
    
    		}
    So once this for loop is done, control_1 has value MAX (which you say is 128). Also, the args_main array has values of 0, from indices 0 to 128. Next:
    Code:
    		do
    		{
    			args_main[control_1] = atoi(argv[control_2]);
    			control_1++; control_2++;
    		} while( atoi(argv[control_2]) >= 1 && atoi(argv[control_2]) <= 9 );
    Your assigning values of 'args_main' with indices 128 to who knows? You probably want to go from 0 to argc. Finally
    Code:
    		control_1 = 0;
    			
    		do						// Checking to make sure user
    		{					       // values were passed to args_main
    			printf("%d  ", args_main[control_1]); // correctly
    			control_1++;
    		} while( control_1 < argc-1 );
    Your printing 'args_main' from indices 0 to argc-1, all of which (as described above) have values 0.

    EDIT: Also, you need to free the memory you allocated with malloc, before the return. Otherwise theres a memory leak of 128bytes each time you run it.

  3. #3
    Registered User jeffcobb's Avatar
    Join Date
    Dec 2009
    Location
    Henderson, NV
    Posts
    875
    Greetings oh foggy one.

    Depending on your platform you could probably benefit from getopt. Getopt automates the parsing and use of command line switches in your application. Here is an example of it in action:
    Code:
    using namespace std;
    // h - help
    // b - binary transfer test
    // m <'m' | 's'>
    // e <NumExchanges>
    // f <filename> is chat file or file to bcast
    // s <servername>
    // p <portname>
    // n <clientname>
    // u <userid>
    // S - flag indicating this is a server (no other switches than the UDP port applies)
    // End of switches
    #define LINUX_OPTS  "Shbm:e:f:s:p:n:u:v"
    struct TOpts
    {
        bool bBroadcaster;
        char cMasterSlave;
        int nExchanges;
        string strFilename;
        string strServerAddr;
        int nPort;
        string strClientName;
        int nUserID;
        bool bIsServer;
        bool bVerbose;
    };
    
    void InitOpts(TOpts *ptrOpts)
    {
        ptrOpts->bBroadcaster = false;
        ptrOpts->cMasterSlave = 's';
        ptrOpts->nExchanges = 10;
        ptrOpts->nPort = 4097;
        ptrOpts->nUserID = 0;
        ptrOpts->strClientName = "ClientX";
        ptrOpts->strFilename = "none";
        ptrOpts->strServerAddr = "dev01";
        ptrOpts->bIsServer = false;
        ptrOpts->bVerbose = false;
    
    }
    
    int TestServer(TOpts *ptrOpts)
    {
        int nRC = -1;
        if(ptrOpts->bVerbose)
        {
            cout << "This will execute a server on port: " << ptrOpts->nPort << endl;
    
        }
        nRC = 0;
        return nRC;
    }
    
    int TestClient(TOpts *ptrOpts)
    {
        int nRC = -1;
        if( ptrOpts->bVerbose )
        {
            cout << "Client test data:" << endl;
            cout << "Client name: " << ptrOpts->strClientName.c_str() << endl;
            cout << "Client Number: " << ptrOpts->nUserID << endl;
            cout << "This client will contact: " << ptrOpts->strServerAddr.c_str() << " on port " << ptrOpts->nPort << endl;
    
            if( ptrOpts->cMasterSlave == 'm' )
            {
                cout << "This is a master client." << endl;
            }
            else
            {
                cout << "This is a slave client." << endl;
            }
            if( ptrOpts->bBroadcaster )
            {
                cout << "This is a broadcast test." << endl;
                cout << "The file to be broadcast is: " << ptrOpts->strFilename.c_str() << endl;
            }
            else
            {
                cout << "This is a chat test. The chat content will be sucking in from file: " << ptrOpts->strFilename.c_str() << endl;
                cout << "This chat test will consist of " << ptrOpts->nExchanges << " exchanges." << endl;
            }
    
    
        }
    
        return nRC;
    }
    
    void Usage()
    {
        cout << "UDP Test tool 1.1" << endl
            << "Switches: " << endl
            << "-b - broadcaster" << endl
            << "-h - help" << endl
            << "-m <m | s> - master or slave" << endl
            << "-f <filename> - file to use for test" << endl
            << "-s <servername> - which server to connect to" << endl
            << "-p <port> - port to use" << endl
            << "-e <exhanges> - how many chat exhanges to do" << endl
            << "-n <clientname> - textual name of this client" << endl
            << "-u <UserNum> - numerical id of client" << endl
            << "-v - verbose" << endl
            << endl;
    
    }
    
    int main (int argc, char *argv[])
    {
        int nResult = -1;
        TOpts Opts;
        InitOpts(&Opts);
        opterr = 0;
        int c;
        while((c = getopt(argc, argv, LINUX_OPTS)) != EOF)
        {
            switch(c)
            {
                case 'b':
                    {
                        Opts.bBroadcaster = true;
                        break;
                    }
                case 'v':
                    {
                        Opts.bVerbose = true;
                        break;
                    }
                case 'h':
                    {
                        Usage();
                        return 0;
                    }
                case 'm':
                    {
                        Opts.cMasterSlave = optarg[0];
                        break;
                    }
                case 'f':
                    {
                        Opts.strFilename = optarg;
                        break;
                    }
                case 's':
                    {
                        Opts.strServerAddr = optarg;
                        break;
                    }
                case 'p':
                    {
                        Opts.nPort = atoi(optarg);
                        break;
                    }
                case 'n':
                    {
                        Opts.strClientName = optarg;
                        break;
                    }
                case 'u':
                    {
                        Opts.nUserID = atoi(optarg);
                        break;
                    }
                case 'e':
                    {
                        Opts.nExchanges = atoi(optarg);
                        break;
                    }
                case 'S':
                    {
                        Opts.bIsServer = true;
                        break;
                    }
                case '?':
                    {
                        // error
                        cout << "Unrecognized switch: -" << optopt << endl;
                        return -1;
                    }
            }
        }
    
        // gets simple from here...
        if( Opts.bIsServer)
        {
            nResult = TestServer(&Opts);
        }
        else
        {
            nResult = TestClient(&Opts);
        }
    
        return(nResult);
    }
    
    This may be overkill in some situations but it does present an elegant way of deaing with switches...
    I like it and is really easy to use once you get the hang of it....
    C/C++ Environment: GNU CC/Emacs
    Make system: CMake
    Debuggers: Valgrind/GDB

  4. #4
    cas
    cas is offline
    Registered User
    Join Date
    Sep 2007
    Posts
    957
    This is a fairly convoluted program, and there are a number of problems. Most basically, you're not using argc to check how many arguments to process. Rather, you just keep on converting argv elements until you find one that is less than 1 or greater than 9. What happens if the user doesn't enter an invalid entry? Well, you wind up calling atoi(NULL) which is invalid (NULL is the last entry in argv). You also seem to be trying to run the program with arguments that are larger than 9, but your do/while loop only accepts arguments between 1 and 9, inclusive. Is that what you want? In addition, after initializing args_main to zeroes, you forget to reset control_1 to zero, so instead of putting the converted argv elements into args_main[0], args_main[1], and so on, you place them beyond the end of the array (since control_1 is 128 after your initialization loop). Also, your use of args_main_p is not right. You don't actually use it for anything, but when you allocate memory with malloc(), you're overwriting its old value, which you appear to have set with a reason.

    I'll show you an example of a cleaner way to do what it seems you're doing:
    Code:
    #include <stdlib.h>
    #include <stdio.h>
    
    #define MAX 128
    
    int main(int argc, char **argv)
    {
      int args_main[MAX] = { 0 }; /* This sets all elements to zero. */
      int i;
    
      for(i = 1; i < argc; i++)
      {
        /* The arguments in argv start at 1, so subtract 1 when storing
         * to args_main so that it starts at 0.
         */
        args_main[i - 1] = atoi(argv[i]);
    
        /* I'm not sure what your criteria are; this stops when a 0 or
         * negative argument is found.
         */
        if(args_main[i - 1] < 1) break;
      }
    
      /* I assume 0 is a terminating value. */
      for(i = 0; args_main[i] != 0; i++)
      {
        printf("%d ", args_main[i]);
      }
      putchar('\n');
    
      return 0;
    }
    There are problems with this code, such as the fact that args_main might overflow (if too many command-line arguments are given), and atoi() is not a safe function. But you get the idea, I hope.

  5. #5
    cas
    cas is offline
    Registered User
    Join Date
    Sep 2007
    Posts
    957
    Quote Originally Posted by jeffcob
    Code:
    while((c = getopt(argc, argv, LINUX_OPTS)) != EOF)
    If I may be ever so pedantic, getopt() returns -1 when it runs out of arguments, and EOF need not be -1. It probably is on most systems, but why risk it?

  6. #6
    Registered User
    Join Date
    May 2009
    Location
    Oregon, USA
    Posts
    9
    Your assigning values of 'args_main' with indices 128 to who knows? You probably want to go from 0 to argc. Finally
    I can't believed I missed that. Go me :P That fixed my problem. Thanks a bunch

  7. #7
    Registered User
    Join Date
    Oct 2006
    Location
    Canada
    Posts
    1,243
    Quote Originally Posted by London Fog View Post
    I can't believed I missed that. Go me :P That fixed my problem. Thanks a bunch
    It happens. However, this is more likely to happen when you have a number of arrays, variables, counters, etc, as you have. I would try to reduce the number of variables you have, if possible, to reduce the probability of having another one of these logic errors.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Assigning Multiple Values to Array/ Vector
    By bengreenwood in forum C++ Programming
    Replies: 2
    Last Post: 07-02-2009, 01:15 PM
  2. Question about assigning values
    By DCMann2 in forum C Programming
    Replies: 7
    Last Post: 04-18-2008, 07:36 AM
  3. Replies: 1
    Last Post: 12-30-2007, 09:08 AM
  4. Sending values to a control
    By Zyk0tiK in forum C Programming
    Replies: 6
    Last Post: 12-02-2005, 05:29 PM
  5. Assigning values to arrays
    By napkin111 in forum C++ Programming
    Replies: 7
    Last Post: 02-26-2003, 07:52 PM

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