Adding a 1-dimensional array to a POSIX Shared Memory Object in C

Question

I am working on a POSIX-based program that shares a memory object between two or more processes, aka a Server - Client program.

I have some issues with the Server side of the program. I know how to map memory using mmap() and how to work with the Object between two different processes/programs. However, I have an issue when adding an array of integers to a shared memory object.

I can use sprintf() to print the contents to the shared memory object, then increase (reallocate) space of the Shared Memory Object by simply saying ptr += strlen(whatstringiamworkingwith);

However, I do not know how to re-allocate memory to the object when it contains an array of integers in C. Here is my program:

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

int *calcCollatz(int n) {
    int solution;

    int *collatzSeq = malloc(sizeof(int));
    int j = 0;

    while (n != 1) {
        if (n % 2 == 0) {
            solution = n / 2;
            n = solution;
            collatzSeq[j] = n;
            j++;
            return collatzSeq;
        } else {
            solution = (3 * n) + 1;
            n = solution;
            collatzSeq[j] = n;
            j++;
            return collatzSeq;
        }
    }
}

int main(int argc, char *argv[])
{
    const int SIZE = 4096;          //size in bytes of Shared Memory Object
    const char *sharedObj = "Shm";  //name of the Shared memory Object
    int shm_fd;                     //Shared memory file descriptor
    void *ptrShm; //Pointer to shared memory object

    shm_fd = shm_open(sharedObj, O_CREAT | O_RDWR, 0666); //Create Shared Memory Object
    ftruncate(shm_fd, SIZE);  //configure the size of the shared memory object
                              //Map the shared memory object in the space of the process
    ptrShm = mmap(0, SIZE, PROT_WRITE, MAP_SHARED, shm_fd, 0);

    if (ptrShm == MAP_FAILED) {
        printf("Map Failed\n");
        return -1;
    }

    sprintf(ptrShm, argv[1]);
    ptrShm += strlen(argv[1]);

    sprintf(ptrShm, calcCollatz(atoi(argv[1])));
    ptrShm += sizeof(calcCollatz(atoi(argv[1])));


    printf("Writing the sequence to a shared memory object!\n");
    return 0;
}

Am I even on the right track?

Answer 1

There are some errors and issues in your code.

1. ptr += strlen(whatstringiamworkingwith); is advancing a pointer, not reallocating memory.

2. Doing sprintf(ptrShm, argv[1]); is dangerous, if I execute your program like this: ./yourprogram %s then you will have undefined behaviour as the %s specifier would be present but a pointer to char* to satisfy the specifier is missing. Never pass a variable filled by the user as the format for printf & Co. So do it like this:

   sprintf(ptrShm, "%s", argv[1]);
   

   Sames goes for this line: sprintf(ptrShm, calcCollatz(atoi(argv[1])));

   And you are not checking if there are enough command line arguments, you should add this at the beginning of your program:

   if(argc != 2)
   {
       fprintf(stderr, "usage: %s data\n", argv[0]);
       return 1;
   }
   

3. Error messages should be printed to stderr, so instead of doing printf("Map Failed\n"); you should do:

   fprintf(stderr, "Map failed\n");
   

   but that's not very informative, you should also print the value of errno like this:

   fprintf(stderr, "Map failed: %s\n", strerrno(errno));
   // or
   perror("Map failed");
   

4. You are not checking the return value shm_open.

5. The exit codes are signed 8 bit values, so doing return -1 is equivalent to return 255.

6. As stated in the shm_open man page:

   man shm_open

   a shared memory object should be identified by a name of the form /somename; that is, a null-terminated string of up to NAME_MAX (i.e., 255) characters consisting of an initial slash, followed by one or more characters, none of which are slashes.

   so the proper name should be

   const char *sharedObj = "/Shm";
   

7. sizeof(calcCollatz(atoi(argv[1]))); is the same as sizeof(int*), that mean the size of a pointer to int. And because sizeof is evaluated on compile-time, the function is actually never executed. You probably want to resize by the number of items. Not only that ptr += size is the incorrect call, you are getting the information completely wrong.

   Your calcCollatz is also wrong, you are allocating only space for one single int, why bother dynamically allocating space for a single int? Also you are doing return collatzSeq right away, again, why even bother dynamically allocating space for a single int? I thinks this is what you're looking for:

   int *calcCollatz(int n, size_t *len) {
       if(len == NULL)
           return NULL;
   
       int *collatzSeq = NULL, *tmp = NULL;
       *len = 0;
   
       while (n != 1) {
           // better than n % 2 == 0
           if (n & 1 == 0)
               n /= 2;
           else
               n = 3 * n + 1;
   
           tmp = realloc(collatzSeq, (*len + 1) * sizeof *collatzSeq);
           if(tmp == NULL)
           {
               free(collatzSeq);
               return NULL;
           }
           collatzSeq = tmp;
   
           collatzSeq[(*len)++] = n;
       }
   
       return collatzSeq;
   }
   

8. I don't really understand why you want to resize the mmap memory. Usually you know right away how much memory you need and if you need to store a dynamically generated array, then calculate the amount of memory you need, then create the shared memory, not the other way round:

   size_t base = 4096;
   size_t arrlen;
   
   int *arr = calcCollatz(atoi(argv[1]), &arrlen);
   if(arr == NULL)
   {
       fprintf(stderr, "failed to do calculation\n");
       return 1;
   }
   
   size_t size = base + arrlen;
   
   shm_fd = shm_open(sharedObj, O_CREAT | O_RDWR, 0666);
   if(shm_fd == -1)
   {
       fprintf(stderr, "Failed to open shared memory: %s\n", strerror(errno));
       return 1;
   }
   ftruncate(shm_fd, size);
   
   ptrShm = mmap(0, size, PROT_WRITE, MAP_SHARED, shm_fd, 0);
   
   if (ptrShm == MAP_FAILED) {
       fprintf(stderr, "Map Failed: %s\n", strerror(errno));
       return 1;
   }
   
   printf("Writing the sequence to a shared memory object!\n");
   sprintf(ptrShm, "%s", argv[1]);
   
   // note that void* arithmetic is a GNU extension
   // otherwise you have to cast to (char*) before doing the
   // the arithmetic
   memcpy(ptrShm + base, arr, arrlen * sizeof *arr);
   
   free(arr);
   close(shm_fd);
   return 0;
   

   But if you insist in resizing the mmap memory later, then you can use mremap, but be aware that this is available in GNU/Linux only and you have to add #define _GNU_SOURCE before including sys/mman.h:

   size_t size = 4096;
   
   // create shared memory + mmap
   ...
   
   // exapanding mmap memory
   size_t arrlen;
   
   int *arr = calcCollatz(atoi(argv[1]), &arrlen);
   if(arr == NULL)
   {
       fprintf(stderr, "failed to do calculation\n");
       close(shm_fd);
       return 1;
   }
   
   size_t newsize = size + arrlen * sizeof *arr;
   
   void *newptrShm = mremap(ptrShm, size, newsize, MREMAP_MAYMOVE);
   
   if(newptrShm == MAP_FAILED)
   {
       fprintf(stderr, "Failed to expand memory: %s\n", strerror(errno));
       free(arr);
       close(shm_fd);
       return 1;
   }
   
   ptrShm = newptrShm;
   
   // resize shared file size
   ftruncate(shm_fd, newsize);
   
   memcpy(ptrShm + base, arr, arrlen * sizeof *arr);
   
   free(arr);
   
   ...
   

   See also: Fast resize of a mmap file and How to portably extend a file accessed using mmap()