Buffer is overrun?

Question

I am trying to turn a vector of char* into an array of char pointer but I get this annoying error and I can't figure out what I'm doing wrong.

char** Parse::toCommand(std::vector<char*> command) {
    char** array = new char* [command.size()];
    for (int i = 0; i < command.size(); i++) {
        array[i] = command[i];
    }

    return array;
}

I get this warning which causes my program to not run.

 Buffer overrun while writing to 'array':  the writable size is 'command.public: unsigned int __thiscall std::vector<char *,class std::allocator<char *> >::size(void)const ()*4' bytes, but '8' bytes might be written.

the char* is actually a c string of course.

The strings in the vector are pieces of a string that was cut up using strtok_s. I got rid of the Null at the end of every string by converting each to a c str using string::copy() to get a non constant c string and using the constructor of std::string to get a regular string. I then popped the back to rid myself of the null.

My end goal is I want to have an array of c strings so that I can pass it to execvp()

for (int i = 0; i < exes.size(); i++) {  //Separate each executable and argument list into vector of char* and push that to 2d vector of char*

        char* arg = exes[i]; //arg = the entire string executable and arguments
        std::vector <char*> argV;

        char* place = NULL;

        ptr3 = strtok_s(arg, " ", &place);

        while (ptr3 != NULL) {

            if (*ptr3 == '"') {//if beginning of remaining char* begins with ", push char*
                std::string temp;
                temp.push_back(*ptr3);
                ptr3 = strtok_s(NULL, "\"", &place);
                temp.append(ptr3);
                temp.pop_back();
                temp.push_back('"');
                char* cstr = new char[temp.size()];
                temp.copy(cstr, temp.size(), 0);
                argV.push_back(cstr);
            }
            else if (*ptr3 == '#') {
                break;
            }
            else {
                std::string temp(ptr3);
                temp.pop_back();
                char* cstr = new char[temp.size()];
                temp.copy(cstr, temp.size(), 0);
                argV.push_back(cstr);
            }
            ptr3 = strtok_s(NULL, " ", &place);
        }

        argV.push_back(NULL);
        args.push_back(argV);
    }

    for (int i = 0; i < args.size(); i++) {
        char** command = this->toCommand(args[i]);
        commands[i] = new COM(command);
    }

argV is a vector<vector<char*>> and

Answer 1

To be clear: this warning is not from the compiler but it's from the Microsoft code analyzer.

I made a complete example that reproduces the warning using Visual Studio 2019. There are no more strings, pointers to pointers and other poor design and the warning is still there.

For me the code is correct and it's simply a bug in the Microsoft code analyzer, or the analyzer is over cautious.

#include <iostream>
#include <vector>

int* toCommand(std::vector<int> command)
{
  int* array = new int [command.size()];
  for (size_t i = 0; i < command.size(); i++) {
    array[i] = command[i];
  }

  return array;
}

int main()
{
  std::vector v{1,2,3};
  int* foo = toCommand(v);
  for (int i = 0; i < 3; i++)
    std::cout << foo[i] << "\n";
}

The warning comes from the Microsoft Code analyzer and it's also displayed directly in the IDE:

This is the simplest example that shows the warning:

int* Foo(int size)
{
  int* array = new int [size];
  array[1] = 1;  // with array[0] = 1; the warning goes away
  return array;
}

warning C6386: Buffer overrun while writing to 'array': the writable size is 'size*4' bytes, but '8' bytes might be written.

Answer 2

The problem is not in the code you are showing but somewhere else. Your code allocates rooms for pointers and then assigns those pointers.

The big question is however: did you really evaluate pros and cons of using char * to represent strings instead of std::string? Looks aiming for troubles...

Also your code already depends on std::vector, why using manually allocated char ** instead of a std::vector<std::string>? With a char** the way you use it there is for example no way you can know how many pointers are there (there is no way to implement size() in a portable way).

EDIT

From comments it seems you need to call execvp. From the man page:

The execv(), execvp(), and execvpe() functions provide an array of pointers to null-terminated strings that represent the argument list available to the new program. The first argument, by convention, should point to the filename associated with the file being executed.

The array of pointers must be terminated by a NULL pointer.

Note the last phrase. You need to have one extra pointer set to NULL so that the command can know where the list of strings terminates.