0

I want to create a shared memory with the size of this structure. Two processes should have access to this struct that is why I put it into shm. There are a few ways to create a shared memory and I´m not sure if my code is correct.

typedef struct {

   char *gamename;
   int numberofplayer;
   int player;
  

} gamedata;

int shm () {

    
    int shmid = shmget(IPC_PRIVATE, sizeof(gamedata), IPC_CREATE | 0644);
    if (shmid<0){
        perror("shmget failed \n"),
        return EXIT_FAILURE;
    }

   
   gamedata* data = (gamedata*) shmat(shmid, 0, 0);
   if(data == NULL){
       perror("shmat failed \n");
       return EXIT_FAILURE;
   }
   
   shmctl(shmid, IPC_RMID, 0);

   shmdt(data);
}
  • 3
    Will your `char *gamename` member also point to somewhere in shared memory? Pointers in one process are likely to be invalid in the other process, unless the processes are forks sharing the same code and the pointer is pointing to data that is valid across the fork. – Ian Abbott Jan 10 '22 at 10:40
  • 2
    It's probably not the main point of your question, but `if(data == NULL)` is wrong, it should be `if(data == (void*) -1)`. Also, in C, there's no need to cast void* to other types. Also, consider data races in your code, creating and attaching shared memory is the easy part ;) – alagner Jan 10 '22 at 10:48
  • @alagner Should the shared memory function be a void function? –  Jan 10 '22 at 10:52
  • @chG134 what I meant is that `gamedata* data = (gamedata*) shmat(shmid, 0, 0);` contains unnecessary typecast, `gamedata* data = shmat(shmid, 0, 0);` should suffice, see [this](https://stackoverflow.com/a/605858/4885321). It's written in the context of malloc specifically, but most point apply in general. – alagner Jan 10 '22 at 10:57
  • 1
    Off topic but... if successful `shm` fails to return a value despite being declared `int shm()`. That's undefined behaviour. – G.M. Jan 10 '22 at 11:00
  • @G.M.: The C standard does not universally make it undefined behavior for a function declared with a return type other than `void` not to return a value. The behavior is undefined only when there is an attempt to use the value. This allows the implementation of, for example, a get/set routine that, depending on a command code it is passed, either gets a stored value and returns it or stores a value and returns nothing. – Eric Postpischil Jan 10 '22 at 11:08
  • @EricPostpischil Sorry, I'd misread the tags and assumed `c++`. Thanks for the correction. – G.M. Jan 10 '22 at 11:10
  • @alagner I was told to typecast the struct pointer but I did not really get it.. I just want to attach the struct to shm. Is it correct afer I made your changes? –  Jan 10 '22 at 11:33
  • @chG134 note that both `shmctl` and `shmdt` can also fail; your code does not check for that either. Aside that (and the -1 mistaken for NULL I have already mentioned) it seems correct. – alagner Jan 10 '22 at 12:26
  • @G.M. what should I change in your opinion? –  Jan 10 '22 at 14:25

0 Answers0