I would like to read and write structs with mmap in C.
I have a function named insert_med which allows the insertion of a new struct med into the mmap and each struct (with a unique key) has to be written in a different position of the array (when a new struct is added, it has to be added in the last empty position of the array).
Two structs med CAN'T have the same key as you can see in the code bellow. The key is unique.
My code isn't working - error messages with the variable "struct map": when declared and when the file is ready to be mmapped - but I don't know why. I think I'm probably doing something the wrong way.
#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))
struct med{
int key;
char name[25];
int quant_min;
int quant;
};
insert_med(int argc, char *argv[]){
int i;
int fd;
int result;
struct map*; /* mmapped array of structs */
/* 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 structs
*/
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 = (struct*)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);
}
struct med m;
struct med s;
int a=0;
printf("Key of med: ");
scanf("%d",&m.key);
//strcmp return "0" when the two strings are equal
for (i = 0; i <=NUMINTS; ++i) {
int a=0;
map[i]=&s;
read(fd,&s,1);
if ((strcmp(&m.key,&s.key))==0){
a++;
printf("Med %d already exits. \n",s.key);
break;
}
}
if (a==0){
printf("Name of med: ");
scanf("%s",&m.name);
printf("Quant. min. of med: ");
scanf("%d",&m.quant_min);
printf("Quant. of med: ");
scanf("%d",&m.quant);
map[i]=&m;
write(fd,&m,1);
printf("Med %d saved. \n",m.key);
}
/* 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);
}
In your code, you have:
You previously defined
struct med
(and notstruct map
), and you tried to define a variable of typestruct map *
but gave it no name. The compiler is right to complain. You probably need something like:Julia commented:
Well, I didn't look much further than the first error, but where's there's one, there are usually many.
For example:
needs the cast changed to
(struct med *)
.is wrong. Semantically,
map[i] = s;
would be correct (copy the data froms
into the mapped array) — but, on second thoughts, the assignment is simply wrong. Don't do it; delete that line.Your
FILESIZE
should probably be a multiple ofsizeof(struct map)
rather than a multiple ofsizeof(int)
.The
read()
onfd
is dubious at best — remove it; the whole point ofmmap()
is to avoid direct file I/O.The key values are integers; you don't use
strcmp()
to compare them, and you simply use:The assignment
map[i]=&m;
is wrong, again, and should be deleted, and thewrite()
is wrong. You have an array,map
, and you access it like you would any other array, and the system handles the I/O behind the scenes. Note that you should always check thatread()
andwrite()
operations worked, but that becomes moot when you delete those operations anyway.You will need to review how values are assigned to the
map
array, and ensure you don't try to read from uninitialized values in themap
array.There may still be other problems; I've not compiled, much less run, the code. But these comments should help you on your way.
Note that the error messages from the compiler should be helping too; you need to learn how to interpret them. Make sure you're compiling with enough warnings and that you're fixing all the warnings. If you use GCC, then
gcc -Wall -Wextra -Werror -std=c11
is a fairly good start (I usually add a bunch of warnings about prototypes and function definitions too — some, but not all, of which are included in the above). Remember, the C compiler knows a lot more about C than you do at this stage of your career; listen to its warnings with care, because you should assume it is correct and you've made a mistake. That's what I do, and I've been coding in C for more than 30 years now.Working code
mm7.c
The created program is
mm7
(memory map 7; there's no significance to 7 other than it is prime, small and makes the name unique).A fair amount of the code is left as written in the question, rather than rewrite from scratch. Note that the code is careful to validate input — it detects and bails out on EOF, rather than blithely continuing and doing nothing useful, for example. It includes an output loop to print the stored data. It uses functions to manage parts of the input. It would be better if all the input were handled in functions.
Example data input file
mm7.data
This simulates terminal input with attempted repeats of the number.
Example run with the data file:
The prompts are all on one line for each actual medication because the input was read from a file, not from the keyboard.
Hex dump of data file
/tmp/mmapped.bin
The
memset()
was added because the hex-dump showed garbage in the data structure — harmless garbage, but garbage nonetheless:Initializing the structure with:
didn't completely eliminate the garbage because of the padding bytes in the structure after the name (which surprised me!):
Working with a non-empty file
The code no longer includes
O_TRUNC
when creating the file, but that makes the hard-coded file name less appropriate (but I've not fixed that). Thefind_num_entries()
function simply does a linear search through the data to find where the list left off last time. More sophisticated code would do a binary search through the data to find where the empty entries start. There's also linear search in the lookup for keys. It would be better to keep the file in sorted order so that a binary search can be used for that, too. Indeed, it might be sensible to have a header record in a production version which had a magic number to identify the file type (if the file doesn't start with the correct magic number, it clearly isn't a file that this program wrote, so its contents shouldn't be interpreted), and to record other details such as number of entries and maybe minimum and maximum key values.You can also make the error reporting simpler using functions analogous to
err_syserr()
, which you can find in answers of mine such as Creating a file with given size.