CODEWITHSUNDEEP
cpointersmallocgetline
chx
chx

Reputation: 11770

How to malloc for getline implementation

I'm trying to add getline support to http-fs-wrapper and I have some malloc problems.

ssize_t _intercept_getdelim(int fd, char **lineptr, size_t *n, int delim)
{
    intercept_t *obj = intercept[fd];
    int counter;
    size_t nc = sizeof(char);

    counter = -1;
    while (obj->offset < obj->size)
    {
        ++counter;
        if (*lineptr) {
            *lineptr = realloc(*lineptr, (counter + 2) * nc);
        }
        else {
            *lineptr = malloc(nc);
        }
        _intercept_read(fd, lineptr[counter], nc);
        if (*lineptr[counter] == delim)
        {
           break;
        }
    }
    *n = counter ? counter + 1 : counter;
    *lineptr[counter + 2] = '\0';
    // Why do we need a *n when the return value is the same??
    return *n;
}

Here's the relevant section of _intercept_read:

size_t _intercept_read(int fd, void *buf, size_t count)
{
    memcpy(buf, obj->ra_buf+bo, count);

When I step through this in gdb, the second iteration throws a SIGSEGV (from memcpy -- it's not the ending \0, it's still inside the loop). I also don't quite get what's the difference between the *n of getline/getdelim and the return value.

Upvotes: 1

Views: 798

Answers (2)

chx
chx

Reputation: 11770

This works:

ssize_t _intercept_getdelim(int fd, char **lineptr, size_t *n, int delim)
{
    intercept_t *obj = intercept[fd];
    int counter = -1;
    char *c, *newbuf;

    *n = 1;
    *lineptr = malloc(*n);
    while (obj->offset < obj->size)
    {
        ++counter;
        if (counter >= *n)
        {
            if ((newbuf = realloc(*lineptr, *n  << 1)))
            {
                *n = *n << 1;
                *lineptr = newbuf;
            }
            else
            {
                return -1;
            }

        }
        c = *lineptr + counter;
        _intercept_read(fd, c, nc);
        if (*c == delim)
        {
           break;
        }
    }
    if (counter > -1)
    {
        *(*lineptr + ++counter) = '\0';
    }
    return counter;
}

Upvotes: 0

matthock
matthock

Reputation: 629

The difference between n and the return value is that n is always the buffer size, but the return value can be -1 for error states per posix spec. You aren't fully handling EOF (it should return -1 if it hits EOF and hasn't read anything yet).

A note, reallocing for every character is fairly inefficient. The standard pattern is to double the buffer size each time it is necessary. This is another way the return value and n can differ, since n is the buffer size, which can be much larger than the read character count it returns.

You also don't need to special case a starting null pointer, realloc internally calls malloc in that case.

buf = realloc(buf...) is an unsafe pattern, realloc can return null, you have to save the realloc result to a temp variable and check it before assigning, otherwise you both leak memory and can reference a null pointer.

I don't think there's actually space for the trailing null you're adding to the buffer at the end there.

Upvotes: 1

Related Questions