fgetline_alloc An involved C function for reading lines of arbitrary length

Question

Prototype:

char *fgetline_alloc( FILE *fp, bool flush, int *nchars, bool eolend );

I realize this C99 function may be a bit more involved than expected, hence asking for reviews before using it for real (it's tested, but not extensively). I'll post the source code first, followed by detailed description (which I removed from the source code). I am leaving it in (simplified) debug mode.

I'm also providing quite a few sample-use examples, at the end of the post.

Briefly, the general idea is to return the input as a dynamically allocated c-string (nul-terminated buffer of chars). flush may be set to true for interactive input validation in a loop, from stdin. eolend specifies whether the result will end with EOL or not. nchars specifies either the max count of chars to be read (positive value), or an initial alloc-ahead size in chars (0 or negative value). It also passes back the count of successfully read chars. For details, please see the Details section below the source code.

If cleared, I intend to also make a wchar_t version of it.

Besides anything else you spot, I'd be interested in your opinions about:

• guarding against passed-in overflow of *nchars (is it possible? if so is it worth it?)
• my approach of preventing int overflow for the params of malloc() and realloc()
• realloc()ing the resulting buf to its exact size at the end (I currently don't)

Thanks in advance to anyone taking the time to review this.

Source Code

/* tab space: 4 (regular, not spaces) */

#include <stdio.h>      // for debug messages
#include <stdlib.h>
#include <stdbool.h>    // C99 - bool, true, false
#include <limits.h>     // C99 - for INT_MAX, etc
#include <errno.h>

#define MYDEBUG 1
#if MYDEBUG
    #define DBGMSG( ... )\
    do{\
        fprintf(stderr, "*** %s(): ", __func__ );\
        fprintf( stderr, __VA_ARGS__ );\
    }while(0)
#endif

// -----------------------------------------------
// Read a line from an already open text-stream, and Return it as a dynamically
// allocated c-string. Return NULL on failure (errno is also set). The caller
// is responsible for freeing the returned c-string.
// NOTE: Description details removed for brevity.
//
char *fgetline_alloc( FILE *fp, bool flush, int *nchars, bool eolend )
{
    errno = 0;

    // sanity checks and early exits
    if ( !fp ) {
        DBGMSG( "invalid stream\n" );
        errno = EDOM;
        goto fail;
    }
    if ( ferror(fp) ) {
        DBGMSG( "stream error\n" );
        goto fail;
    }
    if ( feof(fp) ) {
        DBGMSG( "already at EOF\n" );
        goto fail;
    }

    // This can't handle passed-in wrap-around overflow, but with sane values
    // it quits before passing an overflowed param to malloc() later on. The max
    // initially allocated size is abs(*nchars)+1+1 chars when eolend is true
    // (the 2nd +1 is for `\0'), so here we actually define the sane domain for
    // *nchars' to [INT_MIN+2, INT_MAX-2] inclusive.
    if ( nchars ) {
        if ( *nchars > 0 && 2 > (INT_MAX - *nchars) ) {
            DBGMSG( "int arg too big (*nchars=%d)\n", *nchars );
            errno = EDOM;
            goto fail;
        }
        if ( *nchars < 0 && -2 < (INT_MIN - *nchars) ) {
            DBGMSG( "int arg too small (*nchars=%d)\n", *nchars );
            errno = EDOM;
            goto fail;
        }
    }

    // distinguish fixed mode (true) from alloc-ahead mode (false)
    bool ncharsFixed = ( nchars && *nchars > 0 );

    // # of chars to allocate (either fixed or initial alloc-ahead)
    int _nchars = 128;              // default alloc-ahead (*nchars == 0)
    if ( nchars ) {
        if ( *nchars < 0 ) {        // user-defined alloc-ahead
            _nchars = -(*nchars);
        }
        else if ( *nchars > 0 ) {   //fixed *nchars (no alloc-ahead)
            _nchars = *nchars;
        }
    }

    // initial alloc size depends on eolend (but always +1 for nul-terminator)
    char *buf = malloc( eolend ? _nchars+1+1 : _nchars+1 );
    if ( !buf ) {
        DBGMSG( "failed to alloc %d bytes\n", eolend ? _nchars+1+1 : _nchars+1 );
        goto fail;
    }

    // read chars into buf until EOL/EOF, or until *nchars if in fixed mode
    int i = 0;                  // count of successfully read chars
    int c = EOF;
    while ( (c=fgetc(fp)) != '\n' && c != EOF ) {
        if ( i == _nchars ) {
            // fixed *nchars reached? we are done
            if ( ncharsFixed ) {
                break;
            }
            // alloc-ahead _nchars reached? grow buffer size and keep going
            if ( _nchars > (INT_MAX - _nchars - 1) ) { // guard int overflow
                free( buf );
                DBGMSG( "(_nchars=%d) *= 2 would overflow \n", _nchars );
                errno = ERANGE;
                goto fail;
            }
            _nchars *= 2;
            char *tmpbuf = realloc( buf, _nchars+1 );
            if ( !tmpbuf ) {
                free( buf );
                DBGMSG( "failed to realloc %d bytes\n", _nchars+1 );
                goto fail;
            }
            buf = tmpbuf;
        }
        buf[i++] = c;
    }
    // line longer than *nchars? Put back the last unprocessed c, so subsequent
    // function call will not skip it. NOTE: when in alloc-ahead mode the loop
    // stops ONLY after reading EOL or EOF, thus the following if-check fails.
    if ( c != '\n' && c != EOF ) {
        ungetc(c, fp);
        if ( flush ) {          // if specified, also flush extra chars
            for (int ch; (ch=fgetc(fp)) != '\n' && ch != EOF;)
                ;
        }
    }
    // if specified, append EOL
    if ( eolend ) {
        buf[i++] = '\n';
    }
    buf[i] = '\0';              // nul-terminator

    // At this point, should we realloc() buf to its exact size?
    // if ( !ncharsFixed && i < _nchars) {
        // char *tmpbuf = realloc(buf, _nchars+1);
        // if ( tmpbuf ) {
                // buf = tmpbuf;
        // }
    // }

    if ( nchars ) {             // pass to caller # of read chars
        *nchars = i;
    }
    return buf;

fail:
    if ( nchars ) {
        *nchars = 0;
    }
    return NULL;
}

Details

char *fgetline_alloc( FILE *fp, bool flush, int *nchars, bool eolend );

Arguments:

fp: (FILE *) An already opened input text-stream (it may be stdin).

flush: (bool) true if extra chars should be flushed when the line to be read is longer than positive *nchars. It is ignored when *nchars is negative or 0 (see below). In most cases it should be false, but setting it to true can help in validating interactive input with a loop.

nchars: (int *) Pointer to an integer specifying either a fixed count of chars to be read, or an initial alloc-ahead size (in chars). When non-NULL, it passes-back the count of successfully read chars, excluding nul-terminator (0 on error). Passing *nchars as 0, or nchars as NULL, enables alloc-ahead mode with default initial size (128 chars). To enable alloc-ahead mode with a different initial size, pass *nchars with a negative value (see examples). In alloc-ahead mode the initial size auto-grows (doubled) if need be. Also, in alloc-ahead mode the flush argument is ignored by the function.

eolend: (bool) Determines if the returned c-string will end with EOL (true) or not (false). When looping the function to read multiple lines, this arg helps in getting consistent results regardless of the stream's last line, which may or may not end with EOL (it may end with just EOF).

Remarks:

1. Failed calls of the function can be identified by checking its return value against NULL, or by checking the updated *nchars value against 0. errno is also set either directly by the function or implicitly by the C runtime, thus the caller may additionally use perror().

2. There is no "partial success", meaning that even if partial data have been successfully read when an error occurs, they are discarded and the function fails.

3. Valid domain for *nchars is [INT_MIN+2 to INT_MAX-2] inclusive, but memory allocation may be denied for extreme values. If so, the function fails and a subsequent call to perror() will report "not enough space", or similar.

4. When eolend is true and *nchars is positive (fixed allocation), lines longer than *nchars are truncated to *nchars, followed by EOL as an extra char.

   For example, when:

    int nchars = 2;
    char *str = fgetline_alloc(stdin, flush, &nchars, true);
   

   and then feeding stdin with "12345", will set str to "12\n". Thus the passed-back value of nchars willbe 3 (not the original 2).

Sample Usage Examples

With Text Files:

/* C99 */

// prep
char *input = NULL;
int nchars;
bool flush = true;
bool eolend = true;      // always EOL (false or !eolend means: never EOL)

// Read every line of a text-file, ensuring EOL and outputting the line along with its length

FILE *fp = fopen( "in.txt", "r" );    // any text file
if ( fp ) {
    nchars = 0;          // default alloc-ahead (also causes flush to get ignored)
    while ( (input = fgetline_alloc(fp, !flush, &nchars, eolend)) ) {
        printf( "(%u/%d): %s", strlen(input), nchars, input );
        free( input );
        nchars = 0;      // reset for next call
    }
    fclose( fp );
}

// Store up to MAXLINES lines of a text-file in a custom array of lines, rejecting EOLs

struct Line {
    char *data;
    int len;
} lines[MAXLINES+1] = {{NULL,-1}};    // assuming proper MAXLINES (+1 for sentinel)

FILE *fp = fopen( "in.txt", "r" );    // any text file
if ( fp ) {
    int i = 0;            // lines counter
    nchars = -256;        // 256 chars alloc-ahead (also causes flush to get ignored)
    while ( i < MAXLINES
    && (input=fgetline_alloc(fp, !flush, &nchars, !eolend)) ) {
        printf( "Adding line[%d] (%d chars): %s\n", i, nchars, input );
        lines[i].data = input;
        lines[i].len = nchars;
        nchars = -256;    // reset for next call
        i++;
    }
    fclose( fp );
    printf( "%d lines read\n", i );
    lines[i].len = -1;  // sentinel for lines array
}
// verify and cleanup lines array
for (int i=0; lines[i].len > -1 && i < MAXLINES; i++) {
    printf( "Line[%d] (%d chars): %s\n", i, lines[i].len, lines[i].data );
    free(  lines[i].data );
    lines[i].data = NULL;
    lines[i].len = -1;
}

With stdin (Interactive Input):

/* C99 */

/* Omitting proper checking and freeing for brevity */

// prep
char *input;
int nchars;
bool flush = true;       // flush if line is longer than non-zero positive nchars
bool eolend = true;      // always EOL (false or !eolend means: never EOL)

// Read up to 100 chars (strip EOL, flush extra chars)
nchars = 100;
input = fgetline_alloc(stdin, flush, &nchars, !eolend);

// Read whole line with default alloc-ahead (strip EOL, ignore flush)
nchars = 0;              // also causes flush to get ignored
input = fgetline_alloc(stdin, flush, &nchars, !eolend);

// Read whole line with 64 chars alloc-ahead (strip EOL, ignore flush)
nchars = -64;            // intentionally negative (also causes flush to get ignored)
input = fgetline_alloc(stdin, flush, &nchars, !eolend);

/* Using proper checking and freeing */

// Validation loop (32 chars alloc-ahead, strip EOL, ignore flush)
puts( "Enter phrases (\"end\" without quotes to stop):");
for (;;) {
    nchars = -32;        // intentionally negative (also causes flush to get ignored)
    input = fgetline_alloc(stdin, flush, &nchars, !eolend);
    if ( !input ) {
        perror( "*** fgetline_alloc() failed" );
        break;
    }
    if (input && 0 == strcmp(input, "end") ) {
        free( input );
        break;
    }
    printf( "  %d chars in \"%s\"\n", nchars, input );
    free( input );
}

Answer 1

The macro is a little over-complex. If we're happy to always provide a string literal as format specifier (and we should be, for debugging), then just concatenate the prefix:

#define DBGMSG(format, ... )\
    fprintf(stderr, "*** %s(): " format, __func__, __VA_ARGS__ )

Now it's a single expression, we no longer need the do … while(0) wrapper, and can use the return value normally.

---

Using the sign of one argument to switch behaviours is an awkward design choice. It's too easy for an arithmetic error to cause an unintended switch of mode instead of a more visible, and thus correctable, error (either a log message or failure to allocate).

I would prefer to see an extra bool parameter (or more likely a "flags" argument where we can OR together named constants for the options - that's more readable at call sites). We could possibly abstract that behind a pair of clearly-named wrapper functions that then form the main public interface, but given the other switches, I wouldn't do that (we'd need 8 functions already to cover all the flag combinations).

---

    int _nchars = 128;  

Avoid identifiers with leading underscores, even if you do happen to know the tricky details of what you can legally use in every given context, and what's reserved for the implementation.

In any case, please don't name a variable so similarly to a formal parameter - it causes no end of confusion.