Help with custom getline() function

Question

Can anyone explain to me why this isn't working?

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

char *getline(int lim)
{
    char c;
    int i;
    char *line;
    line = malloc(sizeof(char) * lim);


    i = 0;
    while((c = getchar()) != '\n' && c != EOF && i < lim-1)
    {
        *line = c;
        line++;
        i++;
    }
    *line = '\0';
    printf("%s", line);
    return line;
}

I'm not worried about the return value right now - just the reason as to why printf("%s", line) isn't working.

Thanks!

EDIT: fixed to line = malloc(sizeof(char) * lim); but it is still not working.

Solution: the address of *line was being incremented throughout the function. When it was passed to printf(), *line pointed to '\0' because that's where its adress was incremented to. Using a temprorary pointer that stored the original address allocated by malloc() to *line and then passing that pointer into printf(), allowed for the function to walk up the pointer.

Answer 1

It looks like you're printing a zero-length string.

*line = '\0';
printf("%s", line);

I presume that you want to store what line was originally (as returned from malloc) and print that.

Answer 2

Because you are only allocating enough space for a single character in this line:

line = malloc(sizeof(char));

And that is getting filled with the \0 before your printf statement.

I'm guessing you want to change this line to:

/* Allocate enough room for 'lim' - 1 characters and a trailing \0 */
line = malloc(sizeof(char) * lim);

Or even better:

char *line, *tmp;
tmp = line = malloc(sizeof(char) * lim);

And then use tmp in all of your pointer math, this way line will still point to the start of your string.

And I know it's early in your development, but you'll want to make sure you free() the memory that you malloc().

---

Here is a working version of your function including my suggested changes:

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

char *getline(int lim)
{
    char c;
    int i;
    char *line, *tmp;
    tmp = line = malloc(sizeof(char) * lim);

    i = 0;
    /* NOTE: 'i' is completely redundant as you can use 'tmp',
     * 'line,' and 'lim' to determine if you are going to
     * overflow your buffer */
    while((c = getchar()) != '\n' && c != EOF && i < lim-1)
    {
        *tmp = c;
        tmp++;
        i++;
    }
    *tmp = '\0';
    printf("%s", line);
    return line;
}

Answer 3

You seem to have allocated enough room for only one character. Did you mean the following instead:

line = malloc(lim * sizeof(char));

Also, you don't want to change line after reading each character. Use the following block for your while-loop instead:

*(line + i) = c;
i++;

And finally, to null-terminate the string, use:

*(line + i) = '\0';

Answer 4

You're also overwriting memory you don't own. You're malloc'ing one character, setting *line to c, and then incrementing line and repeating.

Answer 5

Updated-it was a simple typo mistake,but you didn't have to vote me down on it

instead of

     char *line= malloc(sizeof(char));

try

    int space= //number of how many characters you need on the line
    char *line= malloc(sizeof(char)*space);

sorry I meant

   char *line= malloc( sizeof(char)*lim)

Answer 6

You need to understand the concept of pointer, and how it's different from a buffer. In your code, you treat "line" as pointer and buffer at the same time.

Answer 7

Everyone has covered these points already, but here is the whole thing all put together:

Edit: Improved the code a little

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

char *getline(int lim)
{
    char *result = malloc(sizeof(char) * lim); // allocate result buffer

    int i = 0;
    char c;
    char *line = result;
    while((c = getchar()) != '\n' && c != EOF && i < lim-1)
    {
        *line = c;
        line++;
        i++;
    }
    *line = '\0';

    printf("%s", result); // print the result
    return result; // return the result buffer (remember to free() it later)
}

Answer 8

You are Making mistake on two points (but you can say same mistake or two ,its upto you). first your pointer should be incremented like

*(line+i) = c; Due to this reason when you set the Null char at the end of loop you are actually saying the compiler to point the pointer to only this poistion.Hence the pointer is only pointing at Null String not the whole string. As it was constantly being moved in each step of the loop. So when you tried to print that the pointer has nothing to print. So if you change your statement inside the loop for pointer and assign the value to an express address rather then actually moving the pointer then you problem will be solved.

Note. If you change that line then you also need to modify your Null terminator assignment like this; *(line+limit) = '\0';