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";
        //=====================================
}

---

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

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. :(

Quote:

---

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].

Quote:

---

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

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

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?

Quote:

---

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

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! :D

Quote:

---

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

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.

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

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.