Search code examples
cdynamic-memory-allocationmemory-efficient

Program efficiency, allocate memory for both read and write


I wrote a simple program and I would like to know if I wrote that program in a efficient way. The program opens a file for reading and writing and then write with pwrite(I don't want the offset of the file to move with the writing), and reading the file with pread. I am just curious to know if it is necessary to allocate memory twice as I do.

#include <stdio.h>
#include <unistd.h>
#include <string.h>
#include <stdlib.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>

int main(int argc, char const *argv[])
{
    char *write_buf = malloc(14), *read_buf = malloc(14);

    int fd = open("file", O_CREAT | O_RDWR, 0644); /* open a file called file */
    if (fd == -1) {
         perror("write");
         exit(-1); // Error, exit program
    }

    strcpy(write_buf, "Hello, World!"); /* copy string into write_buf */
    pwrite(fd, write_buf, strlen(write_buf), 0); /* pwrite (without moving file pointer) write_buf into fd (file) */
    pread(fd, read_buf, strlen(write_buf), 0); /* pread (without moving file pointer) into read_buf from fd (file) */

    close(fd);
    free(write_buf); free(read_buf);

    return 0;
}

Solution

  • There's a lot going just a little bit wrong here, but in C a little bit wrong is the difference between works fine and crashes all the time. Cleaning it up results in this:

    int main(int argc, char const *argv[])
    {
      const int BUFFER_SIZE = 1024;
      char *buf = malloc(BUFFER_SIZE);
    
      int fd = open("file", O_CREAT | O_RDWR, 0644);
    
      if (fd == -1) {
        perror("write");
        exit(-1);
      }
    
      // Here strncpy() can fail if the buffer is too small, so ensure
      // your buffer size is big enough for any reasonable cases. Test the
      // return value if you're not sure what the input size is going to be.
      strncpy(buf, "Hello, World!", BUFFER_SIZE);
      pwrite(fd, buf, strlen(buf), 0);
      
      // strlen() is the wrong tool here, you need the size of the buffer
      // itself, not the length of whatever's in it.
      pread(fd, buf, BUFFER_SIZE, 0);
    
      close(fd);
    
      // This wouldn't be necesssary if you had: char buf[BUFFER_SIZE]
      free(buf);
    
      return 0;
    }
    

    Learn the difference between strlen() and sizeof, as well as how to use constant sizes consistently through your code.

    The principle here is you want to change one thing and have it ripple through your code correctly.