5
\$\begingroup\$

I am writing a library that will be used as input for any application the library is embedded, let's say, an interactive shell or to read from a FILE *.

I need to know which improvements could be made to the snippets below, if a test snippet with a main() function is needed, please let me know.

reader.h:

#if !defined(READER_H)
#define READER_H

#include <stdint.h> /* For uint32_t */
#include <stdio.h>  /* For FILE *   */

/**
 * Reader struct, must be allocated on stack.
 */
typedef struct _reader {
  FILE    *ifp; /* File input buffer.    */
  char    *buf; /* Raw line buffer.      */
  uint32_t len; /* Line length.          */
  uint32_t mul; /* Raw line multiplier.  */
  uint32_t cur; /* Current char on line. */
} reader_t;

/**
 * Opens a new reader, use NULL as @_path to open stdin.
 * Returns 0 on success.
 */
int r_open(reader_t *_reader, const char *_path);

/**
 * Opens a new reader without a file stream.
 * Returns 0 on success.
 */
int r_open_str(reader_t *_reader, const char *_str);

/**
 * Releases memory on internal components and closes the internal file.
 */
void r_close(reader_t *_reader);

/**
 * Reads a line from the current @_reader instance and prints a prompt.
 * Prompt will not be printed to stdout if the file is not stdin.
 * use the @_prompt when running in interactive mode.
 * Returns the length of the string read in the last pass.
 */
uint32_t r_read(reader_t *_reader, const char *_prompt);

/**
 * Peeks a character without changing the current position index on reader.
 * Returns 0 if there are no characters left on current line.
 */
int r_peekc(reader_t *_reader);

/**
 * Advances a position in the reader and returns the same as r_peekc.
 */
int r_nextc(reader_t *_reader);

/**
 * Use this function when iterating the line, returns whether the marker
 * is at end of line.
 */
int r_at_eol(reader_t *_reader);

/**
 * Use this function when you want to know if both markers are past end.
 * This function should be only used when reading content from a file that is
 * not stdin, use at your own risk.
 */
int r_at_eof(reader_t *_reader);

#endif /* READER_H */

reader.c:

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

#include <reader.h>

/* Open a new reader. */
int r_open(reader_t *_reader, const char *_path) {
  if (!_reader)
    return 1;

  _reader->ifp = (_path)? fopen(_path, "rb"): stdin;
  if (!_reader->ifp)  /* Stop now if not opened. */
    return 1;

  _reader->len = 0;
  _reader->mul = 0; /* Mul is used on this context. */
  _reader->cur = 0;
  _reader->buf = NULL;

  return 0;
}

/* Let's open a simple string. */
int r_open_str(reader_t *_reader, const char *_str) {
  if (!_reader || !_str)
    return 1;

  _reader->ifp = NULL; /* Set ifp to null explicitly. */
  _reader->cur = 0; /* mul is not used on this context. */
  _reader->len = strlen(_str);
  _reader->buf = calloc(_reader->len, 1);
  memcpy(_reader->buf, _str, _reader->len); /* Copy the content. */

  return 0;
}

/* Just close the reader and release its memory. */
void r_close(reader_t *_reader) {
  if (_reader) {
    free(_reader->buf), _reader->buf = NULL;
    if (_reader->ifp && (_reader->ifp != stdin))
      fclose(_reader->ifp);
  }
}

/* Peeks the current character. */
int r_peekc(reader_t *_reader) {
  return (!r_at_eol(_reader))? _reader->buf[_reader->cur]: 0;
}

/* Advance a position on reader. */
int r_nextc(reader_t *_reader) {
  if (_reader)
    ++(_reader->cur); /* Advance no matter what. */
  return r_peekc(_reader); /* It's up to you receive whatever. */
}

/* Checks whether we're at eol on current line. */
int r_at_eol(reader_t *_reader) {
  return (_reader && (_reader->cur >= _reader->len));
}

/* Are we really at eof? MUST use when reading from a file. */
int r_at_eof(reader_t *_reader) {
  /* We cannot and shouldn't get the EOF status of stdin... */
  return (_reader && _reader->ifp) && (_reader->ifp != stdin)
      && feof(_reader->ifp) && (_reader->cur >= _reader->len);
}


#define MAX_BUF_MULT (120)  /* Max buffer multiplier. */
#define MAX_BUF_SIZE (1024) /* The max buffer size.   */
/* Let's read a line with a fancy prompt ;)  *
 * This function overwrites the same buffer. */
uint32_t r_read(reader_t *_reader, const char *_prompt) {
  if (_reader) {
    /* Cut it out if we don't have a file. */
    if (!_reader->ifp || feof(_reader->ifp))
      return 0;
    else if ((_reader->ifp == stdin) && _prompt) /* Print prompt if exists. */
      printf("%s", _prompt);
    /* Reset the buffer if past the max size. */
    if (_reader->mul >= MAX_BUF_SIZE)
      free(_reader->buf), _reader->buf = NULL, _reader->mul = 0;

    _reader->cur = 0; /* Reset the current position.          */
    _reader->len = 0; /* We will receive a new string, right? */
    int c = 0;        /* Store the current character (temp).  */
    while (1) {
      c = fgetc(_reader->ifp);
      if (c == '\n' || c == EOF)
         break;

      /* If we ran out of space for characters. */
      if (_reader->len >= _reader->mul) {
        char *tmp = realloc(_reader->buf, _reader->mul + MAX_BUF_MULT);
        if (!tmp) /* If we could not allocate, just break and return. */
          break;
        _reader->buf = tmp; /* Set the buffer back. */
        _reader->mul += MAX_BUF_MULT;
      }
      _reader->buf[_reader->len] = c; /* Assign the character back. */
      ++(_reader->len);
    }
    /* Do it again, if we're at EOF, 0 will be returned ;) */
    if (!_reader->len)
      return r_read(_reader, _prompt);
    _reader->buf[_reader->len] = 0;
  }

  return (_reader)? _reader->len: 0; /* Return the same length. */
}
\$\endgroup\$
2
\$\begingroup\$

Wow, awesome peace of code!

It is properly formatted and well commented.

Some things I noticed in no particular order:

  • missed NULL check after calloc(_reader->len, 1)
  • _t is POSIX thing, you may decide to stick to CamelCase or add some type prefix
  • use of _ before argument name looks rather unusuall, but I guess it is okay

Apart from that your code is rather solid.

\$\endgroup\$
  • \$\begingroup\$ ¡Nice eye for #1! For #2, I'm used to add an _t at the end of the typedef but I think there is not any problem with the prefix, For #3 that's an old habit :P ¡Thanks for the answer! \$\endgroup\$ – NaCl Apr 20 '18 at 12:09
  • \$\begingroup\$ @NaCl, #if !defined(READER_H) vs #ifndef READER_H \$\endgroup\$ – sineemore Apr 20 '18 at 12:15
  • \$\begingroup\$ Do they behave differently? \$\endgroup\$ – NaCl Apr 20 '18 at 12:18
  • 1
    \$\begingroup\$ stackoverflow.com/questions/8617712/… The latter it my personal favorite, is aligns nicely with next line #define READER_H \$\endgroup\$ – sineemore Apr 20 '18 at 12:21

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.