Skip to main content
added 147 characters in body
Source Link
chux
  • 36.4k
  • 2
  • 43
  • 97

Code has:

             if (feof(stream)) {
                if (!count) {
                    free(line);
                    return NULL;
                }
                /* Return what was read. */
                break;
            }

which, IMO, should always returnThis review comment not applicable as even though code was NULLfeof(stream) when an input error occurs, not just whenI originally processed that as count == 0ferror(stream).

Code has:
             if (feof(stream)) {
                if (!count) {
                    free(line);
                    return NULL;
                }
                /* Return what was read. */
                break;
            }

which, IMO, should always return NULL when an input error occurs, not just when count == 0.

Code has:

             if (feof(stream)) {
                if (!count) {
                    free(line);
                    return NULL;
                }
                /* Return what was read. */
                break;
            }

which, IMO, should always return NULL when an input error occurs, not just when count == 0.

This review comment not applicable as even though code was feof(stream) I originally processed that as ferror(stream).

Code has:
             if (feof(stream)) {
                if (!count) {
                    free(line);
                    return NULL;
                }
                /* Return what was read. */
                break;
            }

which, IMO, should always return NULL when an input error occurs, not just when count == 0.

Review point removed.
Source Link
chux
  • 36.4k
  • 2
  • 43
  • 97

Rather than grow allocation linearly by capacity += page_size, use a geometric growth. I like 0, 1, 3, 7, 15 ... SZIE_MAXSIZE_MAX or capacity = capacity*2 + 1.

Naked Magic numbers

With ATTRIB_NONNULL(1, 2), what is 1 and 2 about?

Use a named macro rather than 1, 2. If not possible, at least comment why 1 ,2.

Rather than grow allocation linearly by capacity += page_size, use a geometric growth. I like 0, 1, 3, 7, 15 ... SZIE_MAX or capacity = capacity*2 + 1.

Naked Magic numbers

With ATTRIB_NONNULL(1, 2), what is 1 and 2 about?

Use a named macro rather than 1, 2. If not possible, at least comment why 1 ,2.

Rather than grow allocation linearly by capacity += page_size, use a geometric growth. I like 0, 1, 3, 7, 15 ... SIZE_MAX or capacity = capacity*2 + 1.

added 44 characters in body
Source Link
chux
  • 36.4k
  • 2
  • 43
  • 97

Naked Magic numbers

With ATTRIB_NONNULL(1, 2), what is 1 and 2 about?

Use a named macro rather than 1, 2. If not possible, at least comment why 1 ,2.

Uncomplicate long lines

// char **const tmp = IO_REALLOC(tokens,
//            sizeof *tokens * (capacity += chunk_size));
capacity += chunk_size;
char **const tmp = IO_REALLOC(tokens, sizeof *tokens * capacity);

Style: little reason to type the return from realloc()

(Further, I like comparing with == clearer than !=. 'I don't know half of you half ...)

        // char **const tmp = IO_REALLOC(tokens, sizeof *tokens * (capacity += chunk_size));
        // if (!tmp) {
        //    IO_FREE(tokens);
        //    return NULL;
        // }
        // tokens = tmp;

        void *tmp = IO_REALLOC(tokens, sizeof *tokens * (capacity += chunk_size));
        if (tmp == NULL) {
            IO_FREE(tokens);
            return NULL;
        }
        tokens = tmp; 

Maybe more review laterI realize now OP's post comes after a recent post. It looks like there is a fair amount of repetition of same issues.

Maybe more review later.

Naked Magic numbers

With ATTRIB_NONNULL(1, 2), what is 1 and 2 about?

Use a named macro rather than 1, 2. If not possible, at least comment why 1 ,2.

Uncomplicate long lines

// char **const tmp = IO_REALLOC(tokens,
//            sizeof *tokens * (capacity += chunk_size));
capacity += chunk_size;
char **const tmp = IO_REALLOC(tokens, sizeof *tokens * capacity);

Style: little reason to type the return from realloc()

(Further, I like comparing with == clearer than !=. 'I don't know half of you half ...)

        // char **const tmp = IO_REALLOC(tokens, sizeof *tokens * (capacity += chunk_size));
        // if (!tmp) {
        //    IO_FREE(tokens);
        //    return NULL;
        // }
        // tokens = tmp;

        void *tmp = IO_REALLOC(tokens, sizeof *tokens * (capacity += chunk_size));
        if (tmp == NULL) {
            IO_FREE(tokens);
            return NULL;
        }
        tokens = tmp; 

I realize now OP's post comes after a recent post. It looks like there is a fair amount of repetition of same issues.

added 44 characters in body
Source Link
chux
  • 36.4k
  • 2
  • 43
  • 97
Loading
Source Link
chux
  • 36.4k
  • 2
  • 43
  • 97
Loading