Simple String Program in C - Not able to print the string

Question

I am not able to print the strings. Need some help on what am I doing wrong and some insight on the best approach in these cases.

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

void getName(char *name);

int main()
{
    char name[256];
    getName(name);
    printf("Name = %s\n", name);
    return 0;
}

void getName(char *name)
{
    char line[256];
    printf("Please enter your name! \n");
    gets(line);
    name = (char *) malloc(strlen(line));
    strcpy(name, line);
}

Answer 1

You don't need to do this, comment it out:

name = (char *) malloc(strlen(line));

You already passed in a buffer for name. You don't need to allocate a second buffer.

Also, you didn't add 1 to the allocation to account for the null terminator, but that's not relevant anyway since the allocation is unnecessary either way.

You could gets directly into name and cut out the line middleman, in fact.

Answer 2

You have over-complicated getname() - it just needs to be:

void getName(char *name)
{
    printf("Please enter your name! \n");
    gets(name);
}

Note that gets is widely considered to be an unsafe function to use, and you should try to get into the habit of using fgets instead.

Answer 3

what am I doing wrong

Others have already pointed out some of the errors, but nobody has shown the right solution yet.

Your code is inherently vulnerable to buffer overflows. The gets() function doesn't let you specify the buffer size, so if you enter more than 256 characters, this will write out of the bounds of your array. That's bad.

What you should do instead is

1. not use a separate function for this (because it has no benefits in this particular case), and
2. call fgets() on your array:

---

#include <stdio.h>

int main()
{
    char name[256];
    fgets(name, sizeof name, stdin);
    printf("Name = %s\n", name);
    return 0;
}

Also, if you ever consider dynamic memory allocation:

1. Do not cast. It's a deadly sin.

2. Check the return value of malloc().

3. You will still want to keep track of the buffer size; how about a function like this?

---

#define NAME_LENGTH 256

char *getName(void)
{
    char *buf = malloc(NAME_LENGTH);
    if (buf == NULL) {
        return NULL;
    }

    fgets(buf, NAME_LENGTH, stdin);
    return buf;
}

But, as I mentioned in the comments, in this case you don't need dynamic memory allocation at all.

Answer 4

I know that pointers can be complicated for beginners. So, forget about pointers. How do you expect the following code should run?

void change(int a)
{
   a = 10;
}

int main()
{
    int a = 20;
    change (a);
    printf("%d", a);  // It prints 20
}

(If you did understand the above code, read on. Otherwise, read about functions and scope of a variable.)

Your code has essentially the same structure, only that instead of an int a, you have a char *name. Observe that you have two different name identifiers here. One is getName's argument, and the other one is a constant pointer (an array identifier) defined in main. Changing the argument's value of a function doesn't change the value of the callers' parameter.