Make a function that reads (string) input

Question

I'm new to programming and I have an assignment to make a function that reads a name. For some reason the following 'solution' does not work and the output is always something with weird chinese? characters. What went wrong?

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

void input(char* a);

int main()
{
    char name[8];

    input(&name);

    printf("%s", name);

    return 0;
}

void input(char* a)
{
    char buff[8];

    scanf("%s", buff);

    *a = buff;
}

You are falling into one of the classical rookie mistakes - trying to use a **local** outside a function (`*a = buff;` does **not** copy the string!) — YePhIcK, Jan 28 '18 at 18:57
@YePhIcK So what would be the correct approach? Maybe use a double pointer ( `void input(char** a);` ) ? — Mike th, Jan 28 '18 at 19:00
On top of everything else said here, consider making that buff/name array larger than 8. Maybe 10-20. It can be a source of future bugs and issues. My own name is longer than that :) — atru, Jan 28 '18 at 19:07
An 8-byte buffer is asking for trouble. `scanf` is asking for trouble. A lot of this is nothing but trouble. Use [`read`](http://en.cppreference.com/w/c/io/fread) where you can limit how much data you read from input. `scanf` can and will explode if the data can't fit. — tadman, Jan 28 '18 at 19:07

tom · Answer 1 · 2018-01-28T19:40:56.720

3

I think the problem is in your

    *a = buff;

statement because buff does not have a life outside of your function. so its memory will be lost.. so it is not safe to try and use buff in this way...

[ But as pointed out by @pablo what *a = buff; will do is copy the address of buff and put it into the memory allocated to a, which is really not what you want to do. ]

below should work and do include return from your function

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

void input(char* a);

int main()
{
    char name[8];

    input(&name);

    printf("%s", name);

    return 0;
}

void input(char* a)
{
   // char buff[8];

    scanf("%s", a);

  //  *a = buff;
    return;
}

one other point is to check if you are sure the name will only be 8 characters long... why not have it as 50 characters?

edited Jan 28 '18 at 19:40

answered Jan 28 '18 at 19:02

tom

1,303
10
17

Your first statement is not quite correct (I know that was the OP's intention), but `*a = buff` is assigning the address pointed to by `buff` in the memory pointed to by `a`. – Pablo Jan 28 '18 at 19:16
`input()` return type is of `void` type then why `return` at last in the `input()` function. – Achal Jan 28 '18 at 19:33
@achal - have a look at https://stackoverflow.com/questions/9003283/returning-from-a-void-function there is a whole question devoted to this issue of return or not from a void function – tom Jan 28 '18 at 19:42

machine_1 · Answer 2 · 2018-01-28T19:05:40.727

1

First off, you should pass the array to the function input() not its address. Secondly, replace scanf("%s", buff) with scanf("%s", a). This will store the string directly in the array you pass to the function.

So, the fixed code should look like:

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

void input(char* a);

int main(void)
{
    char name[8];

    input(name);

    printf("%s\n", name);

    return 0;
}

void input(char* a)
{
    scanf("%s", a);
}

The reason why your code doesn't work is that, you try to assign address of the local array buff to the first element of the array that you pass to the function. This shouldn't even compile or the compiler must issue a warning! If your compiler allows it to pass without any of these, that's a disaster.

Finally, the main function should be declared as int main(void) or int main(int argc, char **argv).

edited Jan 28 '18 at 19:05

answered Jan 28 '18 at 19:00

machine_1

4,266
2
21
42

My compiler actually warns. And I honestly never saw the main(void) concept, including in the many books and online. – atru Jan 28 '18 at 19:02
@atru `int main(void)` is one of the 3 possible 'main` combinations. If your compiler warns about it, then is something wrong with your compiler. Which compiler are you using? – Pablo Jan 28 '18 at 19:10
@Pablo I got that, I was just questioning the need for int main(void) and it's superiority over int main(). The third one without using arguments doesn't makes much sense either. Unfortunately clang but I'm pretty sure I had warnings-only on gcc too.. – atru Jan 28 '18 at 19:12
1

The difference between `int foo();` and `int bar(void)` is that you can pass arguments to `foo`, with `bar` the compiler won't let you pass arguments. – Pablo Jan 28 '18 at 19:15

Achal · Answer 3 · 2018-01-28T19:06:24.513

*a = buff; doesn't copy buff to a. Use strcpy() as

strcpy(a,buff);

complete code :

void input(char* a);
int main()
{
        char name[8];
        //input(&name);/** no need to pass & bcz name itself represents address */
        input(name);
        printf("[%s]", name);
        return 0;
}
void input(char* a) {
        char buff[8];
        scanf("%s", buff);
        //*a = buff; /* not valid **/
        strcpy(a,buff);
}

Pablo · Answer 4 · 2018-01-28T19:27:57.057

This function is a problem:

void input(char* a)
{
    char buff[8];

    scanf("%s", buff);

    *a = buff;
}

buff is local variable that is only valid while input() is running, so returning this variable is wrong.

*a = buff; is also wrong. *a is the same as a[0], that means it is a char. buff is an array of char, so you are assigning a pointer to an array of char to a char variable. That doesn't sound right, it's putting apples in the oranges box. In fact what is happening is that you are assigning the address pointed to by buff in the memory pointed to by a. Your compiler should have warned you about that, don't ignore the compiler warnings, they are there to help you, not annoy you,

void input(char *a)
{
    scanf("%s", a);
}

would be the correct function.

Doing

char name[8];

input(&name);

is wrong, even though the address of name and &name will be the same, but they will have different types. name decays into a pointer, so it is a char*. However &name is a pointer to an array of char, a different type. The compiler should give you a warning like this:

warning: passing argument 1 of bar from incompatible pointer type
note: expected char * but argument is of type char (*)[8]

The correct call is:

input(name);

In general there is one big problem, though: You only declare 8 spaces for the buffer. If the name is longer than 7 characters, you will have a buffer overflow. Instead of using scanf I recommend using fgets instead, because here you have much more control of the input and the memory boundaries.

char name[30];
fgets(name, sizeof(name), stdin);
name[strcspn(name, "\n")] = 0; // removing possible newline

scanf is not always easy to use. A name can be long and have spaces in it. Specially for a beginner, this can be tricky. fgets is easier to use.

score 0 · Answer 5 · answered Jan 28 '18 at 19:03

You don't need to pass the address of name since it's a char array. And when you do the copy in input(), you should use strcpy. The following code works:

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

void input(char* a);

int main()
{
  char name[8];

  input(name);

  printf("%s", name);

  return 0;
}

void input(char* a)
{
  char buff[8];

  scanf("%s", buff);

  strcpy(a, buff);
}

Make a function that reads (string) input

5 Answers5