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);

What is a "bad pointer error"? Always paste the error message and your input so that the behaviour is reproducible. — Zeta, Sep 20 '13 at 12:13
the key array has two entries, and yet you write way paste the second element as soon as password has more than one character. — SirDarius, Sep 20 '13 at 12:14
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. — Nemanja Boric, Sep 20 '13 at 12:14
Why you're using for loop? You create a lot of strings in key instead of 2 and get overflow. — Oleg Olivson, Sep 20 '13 at 12:14
@Zeta - I stepped through the code and I get '0xcccccccc ' when I get to 'key[i] = (char*)malloc(keylen * sizeof(char));' and my input was 'fortification' — Dex Dave, Sep 20 '13 at 12:19

score 16 · Accepted Answer · answered Sep 20 '13 at 12:26

16

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);

answered Sep 20 '13 at 12:26

djgandy

1,125
6
15

Good. More importantly do you understand why your example was crashing? – djgandy Sep 20 '13 at 15:39
1

yes wrong declaration of the pointer and the loop was a mistake too thank you for your help again – Dex Dave Sep 21 '13 at 11:08
why are you casting `malloc()` in C code? also, `scanf()` is unsafe – galois Jun 05 '15 at 08:39
1

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. – djgandy Jun 08 '15 at 09:25

Devolus · Answer 2 · 2013-09-20T12:21:35.790

5

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.

edited Sep 20 '13 at 12:21

answered Sep 20 '13 at 12:16

Devolus

21,661
13
66
113

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? – djgandy Sep 20 '13 at 12:17
1

Yes, that is another problem. I'm not as fast with typing as there are errors in that code. :) – Devolus Sep 20 '13 at 12:19
1

It's the `keylen` loop. – Devolus Sep 20 '13 at 12:26
If the password is more than 2 characters you will over flow the key array. – djgandy Sep 20 '13 at 12:27
@Devolus `"you shouldn't cast malloc"` Why is that? `malloc` returns `void *`, is `void *` automatically promoted to `char *`? – jnovacho Sep 20 '13 at 12:29
1

`void *` is in C automatically compatible and you can hide subtle errors when you cast it. http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc – Devolus Sep 20 '13 at 12:36

Malloc a string array - C

2 Answers2