Problem of a string read in function system() (probably due to a bad malloc)

Question

chain1 = (char *) malloc(5*sizeof(char*) + 1);
                strcat(chain1, "cp mediaPlayer.exe ");
                strcat(chain1, actual->chain);
                strcat(chain1, "\n");

system(chain1);

Hi everyone, i have an issue with my actual string... I'm trying to create a string that will be used in the function system that is called by the shell on my linux but the fact is that it can read my string because it doesn't start with cp mediaPlayer.exe but this --> sh: 1: ��P�Pcp: Then, the error below is sent to me on my shell and i really don't know how to fix it.

sh: 1: ��P�Pcp: not found

I'm on it for 2 days but i just couldn't find a solution if someone can help me ? Thanks by the way !

Answer 1

chain1 = (char *) malloc(5*sizeof(char*) + 1);

This will allocate enough space for five character pointers plus an extra byte. A pointer is not the same as a character, it tends to be either four or eight bytes currently but can actually be any size. You probably want sizeof(char) but, since that's always 1, you can just ignore it in multiplications.

You probably also want a number higher than five since that's not even enough to store your fixed prefix.

In addition, the current content of that allocated block will be arbitrary so strcat is not the first operation you should be performing on it (since that requires a valid C string).

This is almost certainly the cause of your "string starts with rubbish" problem. A strcpy would be better as the first operation, or just use sprintf to do the whole thing at once, rather than a strcpy/multiple-strcat sequence.

And, finally, a command passed to system does not need a trailing newline.

A better method may be something like:

static const char prefix[] = "cp mediaPlayer.exe ";
char *chain1 = malloc(sizeof(prefix) + strlen(actual->chain));
if (chain1 != NULL) {
    sprintf(chain1, "%s%s", prefix, actual1->chain);
}

Space for the terminating \0 is accounted for my using sizeof on the string rather than strlen. The former includes space for the terminator. And note the check for a failed malloc, this is something you should always do if you want robust code. I shouldn't need to mention that, should chain1 end up being NULL at the end of this code block, you should not pass it to system.