0

I need a function who reproduce a basic of the getline function for another program, this function should be able to read a FP properly and without a huge buffer (256 should be enough here) I have 2 problem on the code below - the function start to return wrong info when the buffer is exeeded, for this file for example http://pastebin.com/BXj3SH92 ( ./a.out < file ) - when i call my function without giving it a file it should work just like a cat but it stop after i press the first enter key ( ./a.out )

here is the code:

#define MOAR_BUF 256

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

char            *my_getline(const int fd)
{
  static char   save[MOAR_BUF];
  static int    i = MOAR_BUF;
  int           g;
  int           f;
  char          *save2;

  save2 = NULL;
  g = 0;
  f = 0;
  if (g == 0 && i >= (MOAR_BUF -1))
    {
      i = 0;
      g = read(fd, save, MOAR_BUF + 1);
      save[g] = '\0';
    }
  if (i <= MOAR_BUF && save[i] != '\0')
    {
      save2 = malloc((sizeof(*save2) * MOAR_BUF));
      while(save[i] != '\n' && save[i] != '\0')
        save2[f++] = save[i++];
      save2[f] = '\0';
      if (save[i] != '\0')
        i = i + 1;
    }
  return(save2);
}

int             main()
{
  int           i;
  char          **converted_map;
  char          *map;

  i = 0;
  converted_map = malloc(sizeof(*converted_map) * 30);
  while(map = my_getline(0))
    {
      converted_map[i] = strdup(map);
      printf("%s\n", converted_map[i]);
      i++;
    }
}
Saxtheowl
  • 4,136
  • 5
  • 23
  • 32
  • 1
    your `malloc()` is not completely correct: you multiply `MOAR_BUF` with `sizeof(save2)` which ist `sizeof(char *)`. But that only makes the buffer too large an shouldn't cause any problems – Ingo Leonhardt Jul 19 '13 at 10:29
  • fixed, I realize i dont have to malloc **converted_map by the way I dont know why – Saxtheowl Jul 19 '13 at 10:31

1 Answers1

3
static char   save[MOAR_BUF];

g = read(fd, save, MOAR_BUF);
save[g] = '\0';

You tell read to read MOAR_BUF bytes, and when it succeeds reading that many, the 0-termination writes past the end of the array, invoking undefined behaviour.

You need to tell read to read fewer bytes, MOAR_BUF - 1 for example, or make the buffer larger, MOAR_BUF + 1, for example.

Then, in main,

while(map = my_getline(0))
{
  printf("%s\n", converted_map[i]);
  converted_map[i] = strdup(map);
  i++;
}

you print out converted_map[i] before it is assigned, that's more undefined behaviour. You need to first strdup(map) to converted_map[i], then you can print it.

Daniel Fischer
  • 181,706
  • 17
  • 308
  • 431
  • it work for small file but when MOAR_BUF exceed its value I start to have wrong return – Saxtheowl Jul 19 '13 at 10:41
  • `if (i <= MOAR_BUF && save[i] != '\0')` more out-of-bounds access. Fix the out-of-bounds errors, if it still doesn't work after that, give us an example of what goes wrong. Apart from that, you leak memory, you never `free` any of your `malloc`ed pointers, but that might be just for the code here. – Daniel Fischer Jul 19 '13 at 10:47
  • @Saxtheowl Apart from the out-of-bounds accesses, the fact that you try to print `converted_map[i]` before it is assigned looks like a cause of trouble. – Daniel Fischer Jul 19 '13 at 11:03
  • I dont see what you mean by out of bound access, for the error example when i type ./a.out < map (http://pastebin.com/9vhxzPYG) i get that: http://pastebin.com/5ZN7qx9R – Saxtheowl Jul 19 '13 at 11:07
  • @Saxtheowl You're accessing `save[256]`. But the last valid index is `255`. Is your complaint that some of the lines are split? That is because you never check whether the `save` contents ends with a newline, so whenever the line lengths don't add up to a multiple of the buffer length, you split. I thought that was intentional, since you have no code at all for joining split lines. – Daniel Fischer Jul 19 '13 at 11:17