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
– mukunda, Commented Nov 19, 2014 at 21:55
C strings are null terminated. The size needs to be incremented by 1. — cup
– cup, Commented Nov 19, 2014 at 21:57
Don't cast the result of malloc (and friends). Also, sizeof(char) is always 1. — Deduplicator
– Deduplicator, Commented Nov 19, 2014 at 22:03
don't use gets. it is inherently dangerous and has been removed from the C standard — Jens Gustedt
– Jens Gustedt, Commented Nov 19, 2014 at 22:13

Bob Mazanec · Accepted Answer · 2014-11-19 22:21:18Z

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

Not that this is necessarily the best way to do all this, but it's in the 'theme' of the question.
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.)

mukunda · Accepted Answer · 2014-11-19 22:00:05Z

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.

Bill Lynch · Accepted Answer · 2014-11-19 22:16:23Z

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

Collectives™ on Stack Overflow

string and malloc...whats goes wrong?

3 Answers 3

3 Comments

1 Comment

Comments

Linked

Hot Network Questions