Malloc() memory corruption error after concatenating a string

Guys I'm generating a string which rappresent a path to a file, concatenating a macro and a string. The function is this:

char *userPath(char *username)
{
   char *path = (char*)malloc(sizeof(char) * (strlen(MAILBOXES) + strlen(username) + 1));
   path[0] = '\0';
   strcat(path, MAILBOXES);
   strcat(path, "/");
   strcat(path, username);
   return path;
}

The returned pointer reference a correct string, but after some call to this function the process throws out a very very bad * glibc detected ./mmboxd: malloc(): memory corruption: 0x085310a8 ** with the relative backtrace. I know it's here the problem, since I started having this error once implemented开发者_开发问答 it, and also because the only malloc I use is here. What's wrong with this piece of code?

The +1 should be +2 to take into account the separator you add and the null terminator. And you can omit sizeof(char), which will always be 1.

Here's the problem:

   char *path = (char*)malloc(sizeof(char) * (strlen(MAILBOXES) + strlen(username) + 1));

You're allocating enough memory for a) all the characters in MAILBOXES, b) all the characters in username, and c) the '/' character, but you're forgetting d) the terminating '\0' character! So + 1 should be + 2

There are a few other oddities about your code, but they're not wrong, just things that could be better:

1. You don't need to cast the return value of malloc in C, and some (like me) consider it bad style for various reasons that you're more than capable of Googling.
2. sizeof(char) is always 1 (this is defined in the standard). Some people say to keep it in for symmetry. Some say take it out since it's one. Some say change it to sizeof *path, so that if you change path to a wchar_t * the malloc will correctly adjust to keep allocating the right size.
3. Using strcat to write the first bit of data to a string is potentially inefficient. Why not drop the path[0] = '\0'; line and just use strcpy for the first bit of data?
4. You calculate the lengths of all of the strings, but then you throw them away and use strcat, which will re-traverse the (previously calculated) lengths to find the right spot. If you stored the results of your two strlen calls, you wouldn't need to use strcat and unnecessarily keep recalculating where the end of the string is.
5. Using strcat to append a single character is inefficient.
6. You don't check the return value of malloc for success or failure before you use it.

You don't appear to have allowed space for the zero-terminator. You should be allocating an extra char for that.

I'm assuming that the +1 in the malloc() is for the path separator. Make it +2 and you'll have space for the terminating null character.

When you allocate the "path" string you forgot to add the length of the "/" char that you add between MAILBOXES and username.

It appears you would need to malloc an another byte for zero termination.

You need to allocate one extra byte for null character "\x00" as the string terminator in C strings.

Currently you allocate only one extra byte for / character.

So try +2 instead of +1

You're not budgeting in a char for the terminating null. Your malloc length should be +2, not +1.

Your +1 in the end of malloc() accounts for the /. But you need space for the null character at the end, which is added by strcat(). So it's a +2.

char *path = (char*)malloc(sizeof(char) * (strlen(MAILBOXES) + strlen(username) + 2));