string and malloc...whats goes wrong?

Question

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

int main() {

    int i, *x , size ;
    char name[100] , *y;

    printf("Give name: ");

    /* user gives name */
    gets(name);
    size = strlen(name);


    /* gets memory for exactly the name length */
    y=(char*)malloc(size*sizeof(char));

    for(i=0;i!='\0';i++) {

        /* corywrites the name */
        *(y+i)=name[i];
    }

    printf("%s" ,*y);
}

`for(i=0;i!='\0';i++){` <- you are comparing the iterator, you need to compare the character value -> `for(i=0;name[i]!='\0';i++){` — mukunda, Nov 19 '14 at 21:55
C strings are null terminated. The size needs to be incremented by 1. — cup, Nov 19 '14 at 21:57
[Don't cast the result of malloc (and friends)](http://stackoverflow.com/q/605845). Also, `sizeof(char)` is *always* 1. — Deduplicator, Nov 19 '14 at 22:03
don't use `gets`. it is inherently dangerous and has been removed from the C standard — Jens Gustedt, Nov 19 '14 at 22:13

Bob Mazanec · Answer 1 · 2014-11-19T22:21:18.710

1

You didn't tell us what problems/symptoms you're seeing...but for starters...

test name[i] != '\0' instead of i != '\0'
the length strlen(name) returns does not include the trailing null.
null-terminate y after the for loop:

So...

y=(char*)malloc((size + 1) * sizeof(char));

for(i=0; name[i] != '\0'; i++) {    
    /* copywrites the name */
    y[i] = name[i]; /* changed to array syntax */
 }
 y[size] = '\0';

 printf("%s", y);

edited Nov 19 '14 at 22:21

answered Nov 19 '14 at 21:57

Bob Mazanec

1,121
10
18

Not that this is necessarily the best way to do all this, but it's in the 'theme' of the question. – Bob Mazanec Nov 19 '14 at 22:06
crash..! user scans the name and then the programm crashes – Konstantinos Tsirakos Nov 19 '14 at 22:13
a) Never cast malloc in C. b) Use strcpy rather than rolling your own. (The whole thing could be replaced with strdup on POSIX systems.) – Jim Balter Nov 19 '14 at 22:47

score 0 · Answer 2 · answered Nov 19 '14 at 22:00

If I would copy a string by hand it would look like:

int i;
for( i = 0; source[i]; i++ ) dest[i] = source[i]; // copy characters
dest[i] = 0; // append null terminator

When you allocate the string you need to add 1 for the null terminator, and after you copy the string, you also need to add the null terminator.

You have a problem where you are comparing i to '\0' which is the initial condition and it terminates immediately.

An easier way to copy a string is to use the strcpy function.

`for( i = 0; (dest[i] = source[i]); i++ ) ;` avoids the need for a separate `dest[i] = 0;` ... but seriously, just use strcpy. — Jim Balter, Nov 19 '14 at 22:48

score 0 · Answer 3 · answered Nov 19 '14 at 22:16

You didn't allocate enough memory for y. You need to allocate space for the number of characters in the string plus one more for the null terminating character.
```
y = malloc(size + 1);
```
Your loop condition is broken. You probably are looking to compare the ith value in the string:
```
for (i=0; name[i]!='\0'; i++) {
```
Instead of writing *(y+i), you should just write y[i]. It's more understandable and it has exactly the same semantics.
```
for (i=0; name[i]!='\0'; i++) {
    y[i] = name[i];
}
```
You didn't null terminate y.
```
y[size] = '\0';
```
When printing a string, the argument is expected to be of type char *, not char. So:
```
printf("%s", y);
```

string and malloc...whats goes wrong?

3 Answers3