Malloc a string array - C

Question

I have been trying to understand malloc and strings, can someone help me with this please - I get a bad pointer error

char password[100];
char *key[2]; 
int textlen, keylen, i, j, k, t, s = 0;
printf("password:\n") ; 
scanf("%s",password);
keylen = strlen(password) + 1;
for(i=0; i < keylen; i++)
{ 
 key[i] = (char*)malloc(keylen * sizeof(char));
 strcpy(key[i], password);
}
printf("The key is:\n\t %s", key);

• Zeta: What is a "bad pointer error"? Always paste the error message and your input so that the behaviour is reproducible.
• SirDarius: the key array has two entries, and yet you write way paste the second element as soon as password has more than one character.
• Nemanja Boric: Not sure what are you trying to do. Anyway, if the password string is larger than one character, keylen will be greater than 1, so key[i] will be out of the bounds access.
• Oleg Olivson: Why you're using for loop? You create a lot of strings in key instead of 2 and get overflow.
• Dex Dave: @Zeta - I stepped through the code and I get '0xcccccccc <Bad Ptr>' when I get to 'key[i] = (char*)malloc(keylen * sizeof(char));' and my input was 'fortification'

Answer 1

I think you need to try and understand yourself what you are trying to achieve. You don't need the key[2] array and I think you are confusing yourself there as you don't yet understand how pointers work. The following should work (untested)

// Allow a password up to 99 characters long + room for null char
char password[100];
// pointer to malloc storage for password
char *key; 
int textlen, keylen, i, j, k, t, s = 0;
// Prompt user for password
printf("password:\n") ; 
scanf("%s",password);
// Determine the length of the users password, including null character room
keylen = strlen(password) + 1;
// Copy password into dynamically allocated storage
key = (char*)malloc(keylen * sizeof(char));
strcpy(key, password);
// Print the password
printf("The key is:\n\t %s", key);

• djgandy: Good. More importantly do you understand why your example was crashing?
• Dex Dave: yes wrong declaration of the pointer and the loop was a mistake too thank you for your help again
• galois: why are you casting malloc() in C code? also, scanf() is unsafe
• djgandy: My example is the least work required fix to the original. I did not want to confuse the OP by completely re-writing his entire work. If this was codereview i'd agree with you.

Answer 2

The problem that you have is here:

 printf("The key is:\n\t %s", key);

You are passing the pointer array to printf, while what you would want to do is

 printf("The key is:\n\t %s", key[0]);

Yet another problem is that you allocte as much pointers as you have characters in your password, so you overwrite the pointer array you reserved, because key has only room for two pointers, not for N, which is the primary cause for your "bad pointer" problem.

And another thing, which is not related to this error is, that you shouldn't cast malloc as well as you don't need to multiply with sizeof(char) as this will always be 1 by definition.

• djgandy: You don't think the problem is that if you enter a password with 3 or greater characters key[i] tries to write invalid memory?
• Devolus: Yes, that is another problem. I'm not as fast with typing as there are errors in that code. :)
• Devolus: It's the keylen loop.
• djgandy: If the password is more than 2 characters you will over flow the key array.
• Devolus: void * is in C automatically compatible and you can hide subtle errors when you cast it. stackoverflow.com/questions/605845/…