Allocating and initializing a buffer with file data using a single loop

226 Views Asked by At

I try to read a file to a buffer using a low level file descriptor. The method suppose to store the file data byte by byte to a char * buffer, parse that data, and then free the allocated buffer.

static void
parse_file(char path[11]) {
  int fd = open(path, O_RDONLY);
  if (fd == -1) {
    fprintf(stderr, "Failed to open a file '%s'", path);
    exit(errno);
  }
  char c;
  char *buffer = {0};
  unsigned int i = 0;
  while (read(fd, &c, 1)) {
    buffer = malloc(sizeof(char));  // Why *buffer want work here?
    *buffer = c;
    ++buffer;
    ++i;
  }
  buffer = malloc(sizeof(char));
  *buffer = '\0';
  buffer = &buffer[0];
  printf("Buffer data:\n%s\n", buffer);

  // Parse buffer data
  // ...
  
  buffer = &buffer[0];
  for (unsigned int j = 0; j <= i; ++j) {
    free(buffer);
    ++buffer;
  }
}

I come up with the above solution, but flycheck gives me a warning of unix.Malloc type:

Attempt to free released memory

How can I allocate the buffer char by char in a single loop?

2

There are 2 best solutions below

0
asmie On BEST ANSWER

Construction like buffer = &buffer[0]; won't work. After the loop (and setting \0) buffer points to the last character (so to \0). Taking address of the 0th element will just give you address of the last element (as the buffer points). You cannot 'rewind' to the first character that way. When you call then free() you start freeing from your last element and then iterate over some memory region that was not allocated before.

0
siery On

There are three problems worth mentioning with the above code:

  • *buffer = malloc(sizeof(char)) will not work because *buffer refers to a char type, not a char * pointer.

  • buffer = &buffer[0] will not reset the buffer to it's initial position. To do so, you can either save the address of the first element for later use or move backward through the buffer in a loop using pointer arithmetic.

  • It is more efficient to allocate the buffer after it's size is known.

I ended up using a temporary automatic storage variable with a fixed size and allocating the buffer's memory after the read loop.

static void
parse_file(char path[11]) {
  int fd = open(path, O_RDONLY);
  if (fd == -1) {
    fprintf(stderr, "Failed to open a file '%s'\n", path);
    exit(errno);
  }
  const int MAX = 50;
  char c;
  char *buffer = {0};
  char tmp[MAX];
  unsigned int i = 0;
  while (read(fd, &c, 1)) {
    tmp[i] = c;
    ++i;
  }
  
  buffer = malloc(sizeof(char) * i + 1);
  tmp[i] = '\0';
  strcpy(buffer, tmp);
  printf("Buffer data:\n%s\n", buffer);

  // Parse buffer data
  // ...
  
  free(buffer);
}