Why is my C program not printing out the string?

Question

I am am making a program that prints out what the user types in, and it needs to be done with the method read_line() (for my homework), so I can't change too much.

I don't understand why it isn't printing out what the user enters.

#include <stdlib.h>

char *read_line(char *buf, size_t sz) {
  char tempBuf[sz];
  char c;
  int pos = 0;

  printf("> ");

  while(1) {
    c = getchar();
    if (tempBuf[pos] == EOF || tempBuf[pos] == '\n') {
      buf = tempBuf;
      return buf;
    } else {
      tempBuf[pos] = c;
    }
    pos++;
  }
}

int main(int argc, char **argv) {
  char *buf;
  char *input = read_line(buf, 128);

  printf("Here: %s", input);
}

I am new to C and I am finding it very confusing, so please explain anything in pretty simple terms. Any help would be much appreciated.

Answer 1

char *buf;
char *input = read_line(buf, 128);

Your first line creates a pointer variable called buf but does not assign it any particular value. Your second line passes the value of buf to read_line -- but you never assigned it any particular value. So you passed garbage to read_line and told it to use that garbage as a buffer.

You might want char buf[128]; instead of char *buf;, but it's hard to tell.

Also, please see my comment about your read_line function being broken in a way that indicates that whoever wrote it does not understand how to use getchar.

Answer 2

There are many mistakes in your program.

For starters you are passing to the function an uninitialized pointer

char *buf;
char *input = read_line(buf, 128);

In fact there is no sense to pass the pointer value of which is not used in the function.

Within the function the variable c should be declared as having the type int.

int c;

Otherwise if the type char behaves as the type unsigned char (it depends on a compiler option) a comparison it with EOF will always evaluates to false.

Within the function the array tempBuf is not initialized. So this if statement

if (tempBuf[pos] == EOF || tempBuf[pos] == '\n') {

invokes undefined behavior.

Within the wjile llop you have to check that the value of pos is less than the value of sz.

The function returns a pointer to a local array that makes the returned pointer invalid.

  buf = tempBuf;
  return buf;

Moreover the array even does not contain a string because the terminating zero is not appended to the array.

The function should allocate dynamically memory and return pointer to the allocated memory that will contain a zero terminated string.

Below there is a demonstrative program that shows how the function can be written.

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

char * read_line( size_t n ) 
{
    char *s = malloc( n );

    if ( s != NULL )
    {
        int c;

        printf( "> " );

        size_t i = 0;

        for ( ; i + 1 < n && ( c = getchar() ) != EOF && c != '\n'; i++ )
        {
            s[i] = c;
        }

        s[i] = '\0';
    }

    return s;
}

int main(void) 
{
    size_t n = 128;

    char *input = read_line( n );

    if ( input != NULL ) printf( "Here: %s\n", input );

    free( input );

    return 0;
}

the program output might look like

> Hello Jake Jackson
Here: Hello Jake Jackson

Answer 3

The current implementation of read_line is very flawed due to the simple fact that it returns a locally declared buffer (which gets deleted at the end of the function). As a result, your pointers are pointing at garbage values (which is very dangerous - I repeat - as it will cause your program to crash or worse [keep running even when using memory it doesn't own]).

What you should be doing instead a dynamically creating the buffer on the heap (that way it can safely be returned - but it would have to be manually deleted).

So your function (and the rest of the code) should change to something like the following:

#include <stdlib.h>

char *read_line(size_t sz) {
  // create a new buffer on the heap - so it can easily be returned (note this will have to be deleted after use using free()).
  char *tempBuf = malloc(sz);
  int c;
  int pos = 0;

  // set all elements of tempBuf to nulls
  memset(arr, 0, sz); 

  printf("> ");

  while(1) {
    c = getchar();
    if (tempBuf[pos] == EOF || tempBuf[pos] == '\n') {
      return tempBuf;
    } else {
      tempBuf[pos] = (char) c;
    }
    pos++;
  }
}

int main(int argc, char **argv) {
  char *input = read_line(128);    
  printf("Here: %s", input);

  // free the memory since we are now done with it.
  free(input);
}

You can read more about the heap & stack here.