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;
}
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.