user123
user123

Reputation: 5407

Dynamic memory allocation to char array

I have given the array size manually as below:

int main(int argc, char *argv[] )
{
    char buffer[1024];
    strcpy(buffer,argv[1]);
    ...
}

But if the data passed in the argument exceeds this size, it may will create problems.

Is this the correct way to allocate memory dynamically?

int main(int argc, char *argv[] )
{
    int length;
    char *buffer;
    length = sizeof(argv[1]); //or strlen(argv[1])?
    buffer = (char*)malloc(length*sizeof(char *));
    ...
}

Upvotes: 0

Views: 18496

Answers (6)

Gangadhar
Gangadhar

Reputation: 10526

length= strlen(argv[1]) //not sizeof(argv[1]);  

and

//extra byte of space is to store Null character.    
buffer = (char*)malloc((length+1) * sizeof(char));

Since sizeof(char) is always one, you can also use this:

  buffer = (char*)malloc(length+1);                       

Upvotes: 1

Neil Kirk
Neil Kirk

Reputation: 21803

In C++, you can do this to get your arguements in a nice data structure.

const std::vector<std::string>(argv, argv + argc)

Upvotes: 1

simonc
simonc

Reputation: 42195

sizeof tells you the size of char*. You want strlen instead

if (argc < 2) {
    printf("Error - insufficient arguments\n");
    return 1;
}
length=strlen(argv[1]);
buffer = (char*)malloc(length+1); // cast required for C++ only

I've suggested a few other changes here

  • you need to add an extra byte to buffer for the null terminator
  • you should check that the user passed in an argv[1]
  • sizeof(char *) is incorrect when calculating storage required for a string. A C string is an array of chars so you need sizeof(char), which is guaranteed to be 1 so you don't need to multiply by it

Alternatively, if you're running on a Posix-compatible system, you could simplify things and use strdup instead:

buffer = strdup(argv[1]);

Finally, make sure to free this memory when you're finished with it

free(buffer);

Upvotes: 4

john
john

Reputation: 88027

The correct way is to use std::string and let C++ do the work for you

#include <string>

int main()
{
    std::string buffer = argv[1];
}

but if you want to do it the hard way then this is correct

int main()
{
    int length = strlen(argv[1]);
    char* buffer = (char*)malloc(length + 1);
}

Don't forget to +1 for the null terminator used in C style strings.

Upvotes: 1

Hulor
Hulor

Reputation: 229

Firstly, if you use C++ I think it's better to use new instead of malloc.

Secondly, you're malloc size is false : buffer = malloc(sizeof(char) * length); because you allocate a char buffer not a char* buffer.

thirdly, you must allocate 1 byte more for the end of your string and store '\0'.

Finally, sizeof get only the size of the type not a string, you must use strlen for getting string size.

Upvotes: 0

David Elliman
David Elliman

Reputation: 1399

You need to add an extra byte to hold the terminating null byte of the string:

length=sizeof(argv[1]) + 1;

Then it should be OK.

Upvotes: -1

Related Questions