1

This mmap tutorial from 15 years ago ranks high in Google searches, but it actually runs subtly incorrectly on my Linux system.

mmap_write.c:

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

#define FILEPATH "/tmp/mmapped.bin"
#define NUMINTS  (1000)
#define FILESIZE (NUMINTS * sizeof(int))

int main(int argc, char *argv[])
{
    int i;
    int fd;
    int result;
    int *map;  /* mmapped array of int's */

    /* Open a file for writing.
     *  - Creating the file if it doesn't exist.
     *  - Truncating it to 0 size if it already exists. (not really needed)
     *
     * Note: "O_WRONLY" mode is not sufficient when mmaping.
     */
    fd = open(FILEPATH, O_RDWR | O_CREAT | O_TRUNC, (mode_t)0600);
    if (fd == -1) {
    perror("Error opening file for writing");
    exit(EXIT_FAILURE);
    }

    /* Stretch the file size to the size of the (mmapped) array of ints
     */
    result = lseek(fd, FILESIZE-1, SEEK_SET);
    if (result == -1) {
    close(fd);
    perror("Error calling lseek() to 'stretch' the file");
    exit(EXIT_FAILURE);
    }
    
    /* Something needs to be written at the end of the file to
     * have the file actually have the new size.
     * Just writing an empty string at the current file position will do.
     *
     * Note:
     *  - The current position in the file is at the end of the stretched 
     *    file due to the call to lseek().
     *  - An empty string is actually a single '\0' character, so a zero-byte
     *    will be written at the last byte of the file.
     */
    result = write(fd, "", 1);
    if (result != 1) {
    close(fd);
    perror("Error writing last byte of the file");
    exit(EXIT_FAILURE);
    }

    /* Now the file is ready to be mmapped.
     */
    map = mmap(0, FILESIZE, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
    if (map == MAP_FAILED) {
    close(fd);
    perror("Error mmapping the file");
    exit(EXIT_FAILURE);
    }
    
    /* Now write int's to the file as if it were memory (an array of ints).
     */
    for (i = 1; i <=NUMINTS; ++i) {
    map[i] = 2 * i; 
    }

    /* Don't forget to free the mmapped memory
     */
    if (munmap(map, FILESIZE) == -1) {
    perror("Error un-mmapping the file");
    /* Decide here whether to close(fd) and exit() or not. Depends... */
    }

    /* Un-mmaping doesn't close the file, so we still need to do that.
     */
    close(fd);
    return 0;
}

mmap_read.c:

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

#define FILEPATH "/tmp/mmapped.bin"
#define NUMINTS  (1000)
#define FILESIZE (NUMINTS * sizeof(int))

int main(int argc, char *argv[])
{
    int i;
    int fd;
    int *map;  /* mmapped array of int's */

    fd = open(FILEPATH, O_RDONLY);
    if (fd == -1) {
    perror("Error opening file for reading");
    exit(EXIT_FAILURE);
    }

    map = mmap(0, FILESIZE, PROT_READ, MAP_SHARED, fd, 0);
    if (map == MAP_FAILED) {
    close(fd);
    perror("Error mmapping the file");
    exit(EXIT_FAILURE);
    }
    
    /* Read the file int-by-int from the mmap
     */
    for (i = 1; i <=NUMINTS; ++i) {
    printf("%d: %d\n", i, map[i]);
    }

    if (munmap(map, FILESIZE) == -1) {
    perror("Error un-mmapping the file");
    }
    close(fd);
    return 0;
}

If the file does not already exist, the output of mmap_read is

...
998: 1996
999: 1998
1000: 2000

But if it does, the output is

...
998: 1996
999: 1998
1000: 0

Should the author have flushed the write? Or is GCC miscompiling the code?


Edit: I noticed that it's the prior existence or non-existence of the file that makes a difference, not the compilation flag.

MWB
  • 11,740
  • 6
  • 46
  • 91
  • my inexperience talking here, but isn't O_DIRECT a required flag if you're trying to skip the file write cache? – activedecay Nov 04 '21 at 22:05
  • perhaps it has something to do with O_SYNC and fsync() https://stackoverflow.com/a/49462406/823282 (i'm just throwing darts to see if something sticks) – activedecay Nov 04 '21 at 22:08
  • 4
    Shouldn'e `for (i = 1; i <=NUMINTS; ++i)` be `for (i = 0; i – ikegami Nov 04 '21 at 22:13
  • Have a look at this https://onlinegdb.com/dHQWvfFUi I changed the size to 10 instead of 1000 and I changed the value written so it is alphabetic characters instead of integers and I wrote more than just a \0 at the end of the file. Look at what it writes to the file - it looks like `for (i = 1; i <=NUMINTS; ++i)` starts at integer 1 instead of integer 0 but since it goes to `<=NUMINTS` it writes past the end of the file - overwriting the thing that was previously put at the end of the file. IF you change the for loop it still writes over the byte that was previously put at the end of the file – Jerry Jeremiah Nov 04 '21 at 22:26
  • As I mentioned in my recent answer: https://stackoverflow.com/questions/69813431/copying-files-in-a-directory-using-memcpy-and-mmap-forcing-all-files-to-be-1byte/69814525#69814525 I'd use `ftruncate` instead of `lseek/write` to resize/enlarge the file. This should be done _before_ the `mmap`. Doing `write` _after_ an `mmap` _may_ work, but, IMO, it's hazardous [and unnecessary]. – Craig Estey Nov 04 '21 at 22:26
  • Change 1000 to 1024 and run under `valgrind`. – n. m. could be an AI Nov 04 '21 at 22:37
  • You forgot that indexes in C start form **`zero`** – 0___________ Nov 04 '21 at 22:46
  • Yep, just your basic off-by-one bug. Nothing subtle about writing or flushing or mmap or anything like that. – Nate Eldredge Nov 04 '21 at 23:00
  • @NateEldredge [Double] yep. For extra fun [under _most_ linux systems with 4KB pages], keep the original/broken `for` loop and change to: `#define NUMINTS 1024` [page aligned] and watch for the segfault ... – Craig Estey Nov 04 '21 at 23:05

2 Answers2

6

You are starting at the second element, and writing 2000 after the end of the map.

for (i = 1; i <=NUMINTS; ++i) {
    map[i] = 2 * i; 
}

should be

for (i = 0; i < NUMINTS; ++i) {
    map[i] = 2 * ( i + 1 ); 
}

Demo

It's not a buffering issue. write is a system call, so the data passed to the OS directly. It doesn't mean the data has been written to disk when write returns, but it is in the OS's hands, so it's as if it was on disk as far as OS functions are concerned, including its memory-mapping functionality.

ikegami
  • 367,544
  • 15
  • 269
  • 518
  • yeah, me. Without this change, I get the expected 1998. With this change, I get the expected 2000. – ikegami Nov 04 '21 at 22:35
  • 2
    `write` doesn't buffer. It's a system call. Doesn't mean the data has been written to disk, but it is in the OS's hands. – ikegami Nov 04 '21 at 22:54
1

In C indexes are from zero. Writing and reading index 1000 you invoke undefined behaviour

Change to in the write.:

   for (i = 1; i <=NUMINTS; ++i) {
       map[i - 1] = 2 * i; 
   }

and reading to:

    for (i = 1; i <=NUMINTS; ++i) {
    printf("%d: %d\n", i, map[i-1]);
    }
0___________
  • 60,014
  • 4
  • 34
  • 74