CodeGuru Home VC++ / MFC / C++ .NET / C# Visual Basic VB Forums Developer.com
Results 1 to 12 of 12
  1. #1
    Join Date
    Mar 2003
    Location
    Orange County, CA
    Posts
    99

    Exclamation URGENT: Initializing the size of a char*[]

    I am writing a program that acts as a simple shell. Basically, my program outputs a prompt, then waits for the user to input a command. My program then handles that command and ouputs a new prompt. This process continues until the user quits. The specifics of the program are not relevant though. The only question I have is with the variable char *arg[] inside of my Info struct. I am not sure how to initialize it's size. I tried just saying char *arg[20], but that ended up giving it a size of 0 for some reason.
    Can anyone correct this problem in my code for me?
    Can anyone tell me how to initialize the size of a char*[]?
    I have the entire program below, or you can download the attachment of it.
    Thank You!

    Code:
    /*************************************************
     DESCRIPTION: This program will mimic a shell, displaying a prompt and
    		allowing the user to enter commands. If the commands 'cd' or
    		'exit' are input, the program will handle them. All other commands 
    		will be handled by a child process, which will be forked solely for 
    		the purpose of executing a command, then terminates. The shell will 
    		continue to display a prompt until the user inputs 'exit' or presses
    		CTRL-D.
    *************************************************/
    
    #include <stdlib.h>
    #include <sys/types.h>
    #include <unistd.h>
    #include <wait.h>
    #include <cstring>
    #include <string>
    #include <iostream>
    
    using namespace std;
    
    //Info - holds the information used to execute a user command
    struct Info {
    	char *path;	     //Path to the file
    	char *arg[];      //List of parsed user command
    };
    
    //PURPOSE: Displays a prompt and returns the user input
    //PRECONDITION: None
    //POSTCONDITION: The user input is returned
    void getPrompt(char command[], Info& data);
    
    //PURPOSE: Determines whether a command is intrinsic or not
    //PRECONDITION: None
    //POSTCONDITION: True is returned if command is intrinsic, False otherwise
    int checkIntrinsic(char command[]);
    
    //PURPOSE: Peforms the two intrinisc commands 'cd' and 'exit'
    //PRECONDITION: User inputs the 'cd' or 'exit' command
    //POSTCONDITION: The appropriate task is performed
    void performIntrinsic(char command[]);
    
    //PURPOSE: Sets the path when 'cd' is performed
    //PRECONDITION: User inputs the 'cd' command
    //POSTCONDITION: If a path is specified, it is set to that; 
    //	otherwise, the path is HOME
    void setPath(char command[], char path[]);
    
    //PURPOSE: Parses the user command to be used with exec()
    //PRECONDITION: None
    //POSTCONDITION: The command is parsed and placed in *arg[]
    void parseCmd(char command[], char *args[]);
    
    // **************************************************
    
    
    int main (int argc, char **argv, char **envp) {
    
    	//cmd - holds the command that is input by the user
    	char cmd[200] = "";
    	//status - holds the return status of the forked child
    	int status;
    	//chpid - holds the process ID of the child
    	//ret - holds the status of the wait
    	pid_t chpid, ret;
    	//pInfo - holds the information to be executed in the
    	// child process
    	Info pInfo;
    
    	//Prompt for user input
    	getPrompt(cmd, pInfo);
    
    	//Process the input
    	while (strlen(cmd) != 0 && !cin.eof()) {
    		//If the command is intrinsic, do not fork
    		if (checkIntrinsic(cmd) != 0)
    			performIntrinsic(cmd);
    		//Else, fork a child process and execute the command
    		else {
    			if ((chpid = fork()) == 0) {
    				parseCmd(cmd, pInfo.arg);
    				execvp(pInfo.path, pInfo.arg);
    				//If an error occurs, report it
    				// and exit with a 1
    				perror("Error: command failed ");
    				exit(1);
    			}
    			//wait until the child process has terminated
    			ret = wait(&status);
    		}
    		//Prompt for user input
    		getPrompt(cmd, pInfo);
    	}
    	
    	cout << endl;
    	return 0;
    }
    
    // **************************************************
    
    
    void getPrompt(char command[], Info& data) {
    
    	//Display a prompt
    	cout << "[" << getenv("PWD") << "]$ ";
    
    	//Get the user input
    	cin.getline (command, 200);
    
    	//Remove any leading blank spaces
    	while (isspace(command[0])) {
    		//Move each character forward 1 so the leading
    		// space is overwritten.
    		for (int i = 0; i < strlen(command) - 1; i++)
    			command[i] = command[i + 1];
    
    		//Since the new array is now 1 character shorter,
    		// make the extra element NULL
    		command[strlen(command) - 1] = '\0';
    	}
    	
    	//Load the data structure's path
    	data.path = getenv("PWD");
    }
    
    // **************************************************
    
    
    int checkIntrinsic(char command[]) {
    
    	//Convert the C-string to a string
    	string cmd = command;
    
    	//Check if the command is 'cd'
    	if (cmd.substr(0, 2) == "cd") 
    		return 1;
    	//Else, check if the command is 'exit'
    	else if (cmd.substr(0, 4) == "exit")
    		return 2;
    	//Else, the command is not intrinsic
    	else
    		return 0;
    }
    
    // **************************************************
    
    
    void performIntrinsic(char command[]) {
    
    	//path[] - holds the path used in 'cd'
    	char path[200] = "";
    
    	switch (checkIntrinsic(command)) {
    		//Perform 'cd' command
    		case 1:
    			//Set the path to 'cd' to
    			setPath(command, path);
    			//Now, 'cd' to the path if it exists
    			if (chdir(path) != 0)
    				cout << path << ": No such file or directory.\n";
    			break;
    			
    		//Perform 'exit' command
    		case 2:
    			exit(0);
    	}
    }
    
    // **************************************************
    
    
    void setPath(char command[], char path[]) {
    
    	//index - used to track the index of path[]
    	int index = 0;
    	//specified - flag for if a path is specified in the user command
    	bool specified = false;
    
    	//Check the rest of the command to see if
    	// a path was specified
    	for (int i = 2; command[i] != '\0'; i++) {
    		//If the character is not blank, assume that it is
    		// the part of the path
    		if (command[i] != ' ') {
    			specified = true;
    			path[index] = command[i];
    			index++;
    		}
    	}
    	
    	//If the path was not specified, set it to the HOME directory
    	if (!specified)
    		path = getenv("HOME");
    		
    }
    
    // **************************************************
    
    
    void parseCmd(char command[], char *args[]) {
    
    	//Convert the C-string to a string
    	string cmd = command;
    	//temp - holds the parsed substrings of cmd
    	string temp;
    	//index - used to track the index of *args[]
    	int index = 0;
    
    	//Skip any spaces at the beginning of the string
    	string::size_type lastPos = cmd.find_first_not_of(" ", 0);
    	//Find the first non-space in the string
    	string::size_type pos = cmd.find_first_of(" ", lastPos);
    	
    	while (string::npos != pos || string::npos != lastPos) {
    
    		//Found a token, so place it into the parse array
    		temp = cmd.substr(lastPos, pos - lastPos);
    		strcpy(args[index], temp.data());
    
    		//TEST LINE==============================
    		cout << "args[" << index << "]: " << args[index] << endl;
    		//=====================================
    
    		index++;
    		
    		//Skip the spaces
    		lastPos = cmd.find_first_not_of(" ", pos);
    		//Find the next non-space
    		pos = cmd.find_first_of(" ", lastPos);
    	}
    
    	//TEST LINE==============================
    	cout << "Made it out!\n";
    	cout << "Index is at " << index << endl;
    	cout << "Size of args[] is " << strlen(*args) << endl;
    	//=====================================
    
    	//Add the NULL terminator
    	strcpy(args[index], '\0');
    
    	//TEST LINE==============================
    	cout << "Added NULL!\n";
    	//=====================================
    }
    Attached Files Attached Files

  2. #2
    Join Date
    Apr 1999
    Posts
    27,449
    So where exactly is your problem? In parseCmd? I'm assuming this is what you are referring to.

    Look at this small example program. Why does it print the values of 4 and 7?
    Code:
    #include <cstring>
    #include <iostream>
    
    struct Info {
    	char *path;	     //Path to the file
    	char *arg[20];      //List of parsed user command
    };
    
    void parseCmd(char command[], char *args[]) 
    {
        std::cout << sizeof(args) << std::endl;
        std::cout << strlen(*args) << std::endl;
    }
    
    int main()
    {
        Info INFO;
        INFO.arg[0] = "Testing";
        INFO.arg[2] = "Testing again";
        parseCmd("Test", INFO.arg);
    }
    The sizeof(args) prints 4, because args is a pointer, and the size of a pointer is 4 bytes. The strlen(*args) prints 7 because the *args is the address of the first string in the args array, and that is "Testing".

    So the bottom line is that when you pass a pointer, all your information concerning the number of elements in the array is lost. You cannot recover this information. So either you use a higher-level structure such as a vector of strings (which you can get the size), or you must pass the number of elements separately as another argument to parseCmd.

    If your problem is that the sizeof is not returning the right value, why did you post all your code? If you have a problem, post only that section or ask a specific question. I don't even know if I just answered what you wanted.

    Also, if you wrote a small example program as I have done, you will see why things work the way they do. Many C++ programmers write small, test programs to see how things work.

    Regards,

    Paul McKenzie

  3. #3
    Join Date
    Mar 2003
    Location
    Orange County, CA
    Posts
    99
    Thank you for the comments. I have tested my program and it works if you only input one word (ex: "ls"). But if you input more than one word, it goes out of bounds of the array. So my only question is how do you initialize the size of the *args[ ] array?

    For example, using the exact code that I have, how would you create an instance of the struct Info, with *args[ ] having the size of 20 (i.e. args will be an array of 20 character arrays each of size 20 as well)?

    Someone please help me by answering that question.

  4. #4
    Join Date
    Apr 1999
    Posts
    27,449
    Originally posted by waverdr9
    Thank you for the comments. I have tested my program and it works if you only input one word (ex: "ls"). But if you input more than one word, it goes out of bounds of the array. So my only question is how do you initialize the size of the *args[ ] array?
    Did you see my example? There are 20 char pointers in the args member. I successfully set two strings in the args array my example -- I could have set up to args[19].
    For example, using the exact code that I have, how would you create an instance of the struct Info, with *args[ ] having the size of 20 (i.e. args will be an array of 20 character arrays each of size 20 as well)?
    I answered that question with the example I posted. There is nothing special in your code that would make it follow different rules. If you want an array of 20 pointers, you declare an array of 20 pointers.

    Regards,

    Paul McKenzie

  5. #5
    Join Date
    Apr 1999
    Posts
    27,449
    Also, where is the space that you've allocated for the 20 character pointers? I don't see any call to new[] or malloc() to create this space.

    Again, a simple example:
    Code:
    #include <cstring>
    
    int main()
    {
       char *p1;
       strcpy( p1, "Test");
    }
    This code is in error and may crash your program because p1 is uninitialized and points to who-knows-where. Then you are copying "Test" to who-knows-where, causing a crash.

    Your code does the same thing with the args[] array. So the problem is not the declaration, it is that you have not made room for the characters.
    Code:
    while (string::npos != pos || string::npos != lastPos) {
      //Found a token, so place it into the parse array
    	temp = cmd.substr(lastPos, pos - lastPos);
    	strcpy(args[index], temp.data());
    Look at the last line. Where is the room for args[index] created in your code? I can't find it.

    Also, you want to use c_str(), not data(). See my answer to one of your other posts in another thread.
    Code:
    strcpy(args[index], cmd.substr(lastPos, pos - lastPos).cstr());
    Regards,

    Paul McKenzie

  6. #6
    Join Date
    Mar 2003
    Location
    Orange County, CA
    Posts
    99
    That is what I have been trying to ask all along.
    How do you initialize and allocate the size a char*[], using the 'new' operator?

  7. #7
    Join Date
    Apr 1999
    Posts
    27,449
    Originally posted by waverdr9
    That is what I have been trying to ask all along.
    How do you initialize and allocate the size a char*[], using the 'new' operator?
    Code:
    argv[0] = new char [SomeLength];
    //...
    delete [] argv[0];
    This allocates SomeLength characters for argv[0] and removes it when finished. You do the same thing for argv[1], argv[2], etc. You also have to make sure you free the memory at the end.

    The best way to do this is to create a class that handles these details. Your class should have an AddArg() member function which just adds a string to a string vector. When you need the arguments, another member function, call it GetArgs(), creates the char *[] array from the arguments that are in the string vector, and returns it. The destructor of the class would clean up the memory.

    Regards,

    Paul McKenzie

  8. #8
    Join Date
    Mar 2003
    Location
    Orange County, CA
    Posts
    99
    I attempted to implement your advice in a smaller program:

    Code:
    #include <iostream>
    using namespace std;
    
    class CmdInfo {
    public:
    	CmdInfo(int size);
    	char *path;
    	char *arg[20];
    };
    
    CmdInfo::CmdInfo(int size) {
    	*arg = new char[size];
    }
    
    int main() {
    
    	//Holds the size of the char arrays
    	int cmdSize = 20;
    
    	//Create an instance of CmdInfo
    	CmdInfo pInfo(cmdSize);
    
    	//Assign strings to the first 3 elements
    	pInfo.arg[0] = new char [cmdSize];
    	strcpy(pInfo.arg[0], "ls");
    	cout << "Got 0\n";
    
    	pInfo.arg[1] = new char [cmdSize];
    	strcpy(pInfo.arg[1], "-l");
    	cout << "Got 1\n";
    
    	pInfo.arg[2] = new char [cmdSize];
    	strcpy(pInfo.arg[2], '\0');
    	cout << "Got 2\n";
    
    	//Display the values of the elements
    	for (int i = 0; i < cmdSize; i++)
    		cout << i << pInfo.arg[i] << endl;
    	
    
    	return 0;
    }
    If you compile and run this, however, you will get a segmentation fault when trying to assign a value to index 2 of pInfo.arg[]. Can you fix this little code so that it would work properly? If you can do that, I am sure that I can apply it to my actual program. I am sorry that I am being so difficult in absorbing this concept. It is just foreign to me. Thank you in advance for any more help you can provide. I really appreciate it!

  9. #9
    Join Date
    Apr 1999
    Posts
    27,449
    Originally posted by waverdr9
    I attempted to implement your advice in a smaller program:
    Code:
    CmdInfo::CmdInfo(int size) {
    	*arg = new char[size];
    }
    This only initializes arg[0].
    Code:
    	int cmdSize = 20;
    
    	//Create an instance of CmdInfo
    	CmdInfo pInfo(cmdSize);
    Again, only arg[0] is initialized.
    Code:
    	//Assign strings to the first 3 elements
    	pInfo.arg[0] = new char [cmdSize];
    Why are you doing this again? You already initialized arg[0] in the constructor.
    Code:
    pInfo.arg[2] = new char [cmdSize];
    	strcpy(pInfo.arg[2], '\0');
    	cout << "Got 2\n";
    The strcpy() function takes a pointer to a null-terminates string as the second argument. Why are you specifying a single char?

    There are a lot of things wrong with your code that I haven't covered, but the major thing is that you are not managing the memory correctly. This is where usage of the standard classes is paramount, and only when you need to set up a char *[] do you actually do it. In your code, you do not have a class that encapsulates the argument setting -- instead, you are delegating a lot of the "setting up" and allocating to the main program.

    Here is an implementation of what should be done. Please study it:
    Code:
    #include <string>
    #include <vector>
    #include <iostream>
    #include <process.h>
    
    typedef std::vector<std::string> StringVector;
    
    class ArgHandler
    {
       public:
           ArgHandler() { memset(argv, NULL, sizeof(argv)); }
          ~ArgHandler() { RemoveArgs(); }
    
          void AddArg( const std::string& arg )
          {
              if ( m_arg.size() < 20 )
                 m_arg.push_back( arg );
          }
    
         char **GetArgs() 
         {
             RemoveArgs();
             return CreateArgList();
         }
    
         std::string GetArg(int nArg) const
         {
             if ( nArg >= m_arg.size() )
                 return "";
             return m_arg[nArg];
         }
    
         int GetNumArgs() const { return m_arg.size(); }
         
         private:
             ArgHandler(const ArgHandler& rhs) { }
             ArgHandler& operator = (const ArgHandler& rhs) { }
             void RemoveArgs()
             {
                 int nSize = m_arg.size();
                 for ( int i = 0; i < nSize; ++i )
                 {
                     delete [] argv[i];
                     argv[i] = NULL;
                 }
             }
        
              char **CreateArgList()
              {
                 int nSize = m_arg.size();
                 for ( int i = 0; i < nSize; ++i )
                 {
                     argv[i] = new char [m_arg[i].length() + 1 ];
                     strcpy( argv[i], m_arg[i].c_str());
                 }
                 return argv;
              }
             
              StringVector m_arg;
              char *argv[20];
    };
    
    using namespace std;
    void parseCmd(char command[], ArgHandler& AH) 
    {
    	//Convert the C-string to a string
    	string cmd = command;
    	//temp - holds the parsed substrings of cmd
    	string temp;
    	//index - used to track the index of *args[]
    	int index = 0;
    
    	//Skip any spaces at the beginning of the string
    	string::size_type lastPos = cmd.find_first_not_of(" ", 0);
    	//Find the first non-space in the string
    	string::size_type pos = cmd.find_first_of(" ", lastPos);
    	
    	while (string::npos != pos || string::npos != lastPos) {
    
    		//Found a token, so place it into the parse array
    		AH.AddArg(cmd.substr(lastPos, pos - lastPos));
    
    		//TEST LINE==============================
    		cout << "args[" << index << "]: " << AH.GetArg(index) << endl;
    		//=====================================
    
    		index++;
    		
    		//Skip the spaces
    		lastPos = cmd.find_first_not_of(" ", pos);
    		//Find the next non-space
    		pos = cmd.find_first_of(" ", lastPos);
    	}
    
    	//TEST LINE==============================
    	cout << "Made it out!\n";
    	cout << "Index is at " << index << endl;
    	cout << "Size of args[] is " << AH.GetNumArgs() << endl;
    	//=====================================
    }
    
    int main()
    {
        ArgHandler AH;
        parseCmd("Testing the arg list", AH);
        // Creates a char** of the arguments.
        char **p = AH.GetArgs();
        for ( int i = 0; i < AH.GetNumArgs(); ++i )
            cout << *(p + i) << endl;
        execvp("Testing", p);
    }
    Note the main() program and the parseCmd function. You see no calls at all to allocate any memory. What you do see is a public member function of the ArgHandler class called AddArg(). How it does what it does is no business of the parseCmd() function, it is the responsibility of the ArgHandler class.

    Look how AddArg() is implemented -- all it does is add a string to a string vector. There is no concern for the char*[] array because there is no need for it when you add an argument. The only time when you do need a char *[] is when you actually need it for the execvp() call in your program. In this case, another member function, GetArgs() creates and returns a char **.

    The other thing to note is that when the ArgHandler class is destroyed, the memory is cleaned up (see the ArgHandler destructor).

    Regards,

    Paul McKenzie

  10. #10
    Join Date
    Aug 2002
    Posts
    78
    FYI, his example, while demonstrating a solution, has a few real-world problems:

    1. Use of magic size number "20" in two places -- should be a constant or #define (and possibly not in the class file depending on what else demands 20 be the max size!)

    2. If AddArg is called between calls to GetArgs, RemoveArgs will remove the wrong number of array entries.

    RemoveArgs might be better written as:

    Code:
    {
         for (i = 0; i < 20; i++)  // Use a constant please instead of 20!
              {
                   if (argv[i])
                        {
                             delete [] argv[i];
                             argv[i] = NULL;
                        }
              }
    }
    with an optional break in the else of the "if" if one is sure that, once the first NULL is reached in argv, there will be no more. This code also doesn't rely on the logic of AddArg to maintain the 20 size limit correctly.

  11. #11
    Join Date
    Apr 1999
    Posts
    27,449
    Yes, you're right, but I wrote the code in a flash and did not consider any "fine points" i.e. #define or any thorough debugging. Posting "real-world" code would have taken too much of my time and hopefully the OP corrects the problem.

    Anyway, I believe this was a homework problem, since the very same question was asked just two days ago by someone else.

    Regards,

    Paul McKenzie
    Last edited by Paul McKenzie; October 30th, 2003 at 06:39 PM.

  12. #12
    Join Date
    Aug 2002
    Posts
    78
    I meant to sound more defferential!

    Anyway, from the tone of the original poster, it sounded as if he was looking for a solution to use in a real project, and that he would just cut and paste a working solution into it.

    Then again, if it was a "do my homework" kind of question, they deserve what they get.

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •  





Click Here to Expand Forum to Full Width

Featured