3
\$\begingroup\$

Note: I wasn't trying to follow the POSIX getdelim signature exactly. I needed to add the consume argument for a project I'm working on.

I am pretty new to writing C. It would be great to get some feedback on whether best practices are being followed.

Some things I learned about while writing this code (which might be areas that need improvement)

  • Pointer arithmetic
  • FILE * error handling
#include <errno.h>
#include <stdbool.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

ssize_t read_until_deliminator(char **buffer, size_t *size, char deliminator, 
                               FILE *file, bool consume)
{
        char ch;
        char *buffer_pos;

        if (buffer == NULL || size == NULL || ferror(file)) {
                errno = EINVAL;
                return -1;
        }

        if (buffer == NULL || *size == 0) {
                // Empty buffer supplied
                *size = 128;
                *buffer = malloc(*size);

                if (*buffer == NULL) {
                        errno = ENOMEM;
                        return -1;
                }
        }

        buffer_pos = *buffer;

        for (;;) {
                ch = getc(file);

                if (ch == EOF) {
                        break;
                }

                if ((buffer_pos + 1) == (*buffer + *size)) {
                        // No more room in buffer
                        size_t new_size = *size * 2;
                        char* realloc_buffer = realloc(*buffer, new_size);

                        if (realloc_buffer == NULL) {
                                errno = ENOMEM;
                                return -1;
                        }

                        buffer_pos = realloc_buffer + (buffer_pos - *buffer);
                        *buffer = realloc_buffer;
                        *size = new_size;
                }

                *buffer_pos++ = ch;

                if (ch == deliminator) {
                        if (!consume) {
                                // If not consuming delim roll back buffer
                                buffer_pos--;
                        }

                        break;
                }
        }

        if (ch == EOF && buffer_pos == *buffer) {
                return -1;
        }

        *buffer_pos = '\0';

        return buffer_pos - *buffer;
}

int main(void)
{
        size_t buffer_size = 50;
        char *buffer = malloc(buffer_size);

        ssize_t result = read_until_deliminator(&buffer, &buffer_size, 'c', stdin, false);

        printf("Result: %zd\n", result);
        printf("Buffer Size: %zu\n", buffer_size);
        printf("Buffer: %s\n", buffer);

        free(buffer);
}

Minimal working example: https://repl.it/repls/SlimEarlyField

\$\endgroup\$
4
\$\begingroup\$

Good job!

A few notes:

  1. It is better to check for a malloc error right away instead of going to another function to check. Just like what you did with realloc.
  2. Your function read_until_deliminator returns an error but your implementation never checks for this error return value.
  3. I will do separation of concern, where you will obtain the input and find the delimiter in different functions. This is just to aid in code maintenance so it helps with readability for you and others.

I will post my changes later in the day.

\$\endgroup\$
2
\$\begingroup\$

If we pass a null buffer, expecting it to be allocated for us, then we'll be surprised, because the first check will return a failure here:

    if (buffer == NULL || size == NULL || ferror(file)) {
            errno = EINVAL;
            return -1;
    }

    if (buffer == NULL || *size == 0) {

Perhaps those conditional blocks need to be in the opposite order? And the buffer/size check to be && rather than ||?

Kudos for using realloc() correctly (testing the result before assigning to *buffer - you've avoided a common mistake there.

Minor (spelling) - "delimiter", not "deliminator".

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.