HiveBrain v1.2.0
Get Started
← Back to all entries
patterncMinor

Reading a line ending with a newline character from a file descriptor

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
fromreadingfilenewlinelinewithdescriptorcharacterending

Problem

I'm implementing a function that behaves like getline() which reads a line from file descriptor and returns the results without \n. allowed functions, read(), free() and malloc. Please review my code.

```
#include
#include
#include
#ifndef GET_C_BUFF_SIZE
# define GET_C_BUFF_SIZE 1023
#endif
#ifndef BUFF_SIZE
# define BUFF_SIZE 32
#endif

void my_memcpy(void dest, const void *src, size_t n)
{
unsigned char *n_dest;
const unsigned char *n_src;

n_dest = (unsigned char *)dest;
n_src = (unsigned char *)src;
while (n--)
n_dest++ = n_src++;
*(++n_dest) = '\0';
return (dest);
}

void my_realloc(void ptr, size_t len)
{
void *real;

real = malloc(len);
if (real)
my_memcpy(real, ptr, len);
free(ptr);
return (real);
}

int my_getchar(const int fd)
{
static char buff[GET_C_BUFF_SIZE];
static char *chr;
static int pos = 0;
static int ret = 0;

if (pos >= ret)
{
if ((ret = read(fd, buff, GET_C_BUFF_SIZE)) > 0)
{
chr = buff;
pos = 0;
return (*(chr + pos++));
}
else
return (0);
}
else
return (*(chr + pos++));
}

int read_line(char *text, int buf_size, char **line, const int fd)
{
int position;
int c;

position = 0;
while (1)
{
c = my_getchar(fd);
if (c == 0 || c == '\n')
{
text[position] = '\0';
*line = text;
return (1);
}
else
text[position] = c;
position++;
if (position >= buf_size)
{
buf_size += BUFF_SIZE;
text = my_realloc(text, buf_size);
if (!text)
return (-1);
}
}
return (1);
}

int get_line(const int fd, char **line)
{
char *text;
int buf_size;

buf_size = BUFF_SIZE;
text = malloc(sizeof(char) * buf_size);
if (fd < 0 || !text

Solution

Static buffer

In my_getchar(), you use a static buffer buff along with some other static variables. The problem with this is that you can no longer call get_line() on two different files at the same time. For example, if you did this:

get_line(fd1, &line1);
get_line(fd2, &line2);


Then the second get_line() would actually read characters from the first file descriptor, because you have buffered its contents in the static buffer.

Reallocation strategy

Your current reallocation strategy is to increase the size of the buffer by BUFF_SIZE when you run out of space. This causes your function to have a \$O(n^2)\$ time complexity. It would be better if you doubled the size of your buffer instead of adding a constant to it.

Code Snippets

get_line(fd1, &line1);
get_line(fd2, &line2);

Context

StackExchange Code Review Q#135133, answer score: 3

Revisions (0)

No revisions yet.