2

I created this program to pass a message to a parent process. I want the parent process to print out the message it receives. I'm not sure if this is an issue with reading the char array or message passing as I am quite new to programming in c. Here is my attempt:

struct msg {
        long int    mtype;      /* message type */
        char        mtext[1028];   /* message text */
} msg;
int pid, len;
int msgflg = 0;
int msqid;
char *mymsg[1028];
size_t msgsz;
long int msgtyp;

switch(pid=fork()) //fork child process
{//Child process
case 0:
    mymsg[1] = "serving for sender\n";
    len = strlen(mymsg[1]);
    msgsnd(msqid,mymsg[1],len,msgflg);
    break;
case -1:
    printf("fork failed");
    exit(-1);
    break;
default:
    msg.mtype = 0;
    msqid = msgget(IPC_PRIVATE,msgflg); 
    wait((int *) 0);
    msgrcv(msqid,&msg,msgsz,msgtyp,IPC_NOWAIT);

    printf("%s",msg.mtext);
    msgctl(msqid, IPC_RMID, NULL);
    exit(0);
}

My question is, why is the message serving for sending not being displayed when this code is compiled and executed?

Matt_Bro
  • 14,062
  • 2
  • 28
  • 41
  • You should probably create the message queue before you fork. You can't use an uninitialized `msqid` and expect to send a message. You should also religiously check the return status of the system calls; you need to know when things are going wrong. – Jonathan Leffler Sep 30 '11 at 03:40

3 Answers3

3

You haven't really asked a question, but a couple of issues I can see with the code are:

char *mymsg[1028];
...
mymsg[1] = "serving for sender\n";

Here you have mymsg which is an array of 1028 pointers to char, which is intended to be treated as a string. (By the way, why 1028? Not that it matters, but just so you know 2^10 is 1024). However, this array contains pointers that are not initialized and are pointing to random locations. Important thing is, there is no space allocated for the possible message you want to put in them.

Second issue is that arrays in C start with index 0, so you probably meant to write

mymsg[0] = "serving for sender\n";

That doesn't matter however.

More importantly, you can't copy strings in C using =, you should use strcpy and copy to a memory location that you have already allocated. Here are two ways to do it:

char mymsg[1028][1028];    // if you are sure your messages fit in 1028 chars
...
mymsg[1] = malloc(strlen("serving for sender)*sizeof(char)); // sizeof(char) not really needed
strcpy(mymsg[1], "serving for sender\n");
msgsnd(msqid,mymsg[1],len,msgflg);
free(mymsg[1]);

or

char *mymsg[1028];
...
char str_to_be_printed[] = "serving for sender\n";
mymsg[1] = malloc(strlen(str_to_be_printed)*sizeof(char)); // sizeof(char) not really needed
strcpy(mymsg[1], str_to_be_printed);
msgsnd(msqid,mymsg[1],len,msgflg);
free(mymsg[1]);

Edit: In the second case where you already have the string somewhere (and not in the form of "this is a string"), assigning the pointers is enough and you don't to copy or allocate memory. However, if your situation is more complex than this, and between assignment of mymsg[1] = ... and msgsnd there are other code, you have to make sure the original string stays alive until msgsnd is done. Otherwise, you have a dangling pointer which will cause you problems. Here's the idea:

                        +-+-+-+-+-+-+-+-+--+
str_to_be_printed ----->|A| |s|t|r|i|n|g|\0|
                        +-+-+-+-+-+-+-+-+--+
                        ^
mymsg[1]---------------/

If you free the memory of str_to_be_printed, access to mymsg[1] would cause segmentation fault/access violation.

Note that, the code I wrote is just to give you a guideline, don't copy-paste it.

Shahbaz
  • 46,337
  • 19
  • 116
  • 182
  • 1
    I think in the code `=` is being used to assign the pointer to char array to a string which will be created in read-only memory section. OP is not copying anything. Assignment seems legit. – another.anon.coward Sep 30 '11 at 04:13
  • I had considered that and I will edit my answer. However, since the OP said he was new to C, I thought it may not be a bad idea to make sure he understood that. – Shahbaz Sep 30 '11 at 10:30
  • @matt_bro did you get the answer to your question? Do you need more help? – Shahbaz Oct 03 '11 at 15:06
1

There are few observation related to your code.

  1. When you use system calls (any function which returns any error code) check the return value of the function. In the case of system calls you have used will set errno i.e. error number which can be used to check for error. You can use perror or strerror to see the message (Pointed out by Jonathan Leffler already)
  2. You need to create message queue before using it (Again pointed out by Jonathan Leffler already).
  3. You are sending char * in msgsnd & receiving struct msg type in msgrcv.
  4. You have set the size to be passed in the message queue send & receive calls, for which you are using msgsz but uninitialized. Set value of msgsz to the size you want to send/receive. While sending you seem to sending 17 bytes but while receiving it is not set.
  5. mtype should have a value greater than 0.
  6. Type for pid should be pid_t, which seems do you little good in this case anyway.

Some code sections for your reference:

#include <stdio.h> /*For perror*/
...
/* Create message queue */
msqid = msgget(IPC_PRIVATE, IPC_CREAT);
if( 0 > msqid )
{
 perror("msgget");
 /* Handle error as per your requirement */
}

... 
/* Sending & receiving messages */
...
struct msg {
        long int    mtype;      /* message type */
        char        mtext[1028];   /* message text */
} sender_msg, receiver_msg;
...

size_t msgsz = 10; /* Send & receive 10 bytes, this can vary as per need. You can receive less than what was sent */
...
switch(fork())
{

case 0: /* Child */
    sender_msg.mtype = 1;
    strncpy(sender_msg.mtext,"01234567890123", 1027);
    /* Sending the whole text size */
    if ( 0 > msgsnd(msqid, &sender_msg, strlen(sender_msg.mtext),msgflg))
    {
      perror("msgsnd");
      /* Handle error as per your requirement */
    }
break;

case -1:
    perror("fork");
    exit(-1);
break;
default:
    wait((int *) 0);
    receiver_msg.mtype = 1;
    /* Receive only 10 bytes as set in msgsz */
    if( 0 > msgrcv(msqid,&receiver_msg,msgsz,msgtyp,IPC_NOWAIT))
    {
      perror("msgrcv");
      /* Error handling */
    }
    printf("%s",receiver_msg.mtext);
    if (0 > msgctl(msqid, IPC_RMID, NULL))
    {
      perror("msgctl");
      /* Handle error as per your requirement */
    }
break;

}

You seem to be using System V message queues APIs here, you can look into the POSIX message queue APIs like mq_open, mq_close, mq_send, mq_receive etc. For message queue overview see the man pages (man mq_overview)
Use man pages for information about APIs as well.
Hope this helps!

another.anon.coward
  • 11,087
  • 1
  • 32
  • 38
1

You have several problems:

  • You need to create the message queue before you call fork(), so that both the parent and child have access to it;

  • The permissions of the message queue are set from the low-order bits of the second parameter of msgget(), so you need to specify at least read and write permissions for the owner of the message queue. You can use the constant S_IRWXU from <sys/stat.h> here;

  • You are passing msgsnd() a pointer to a string, but it actually wants a pointer to a message struct like your struct msg.

  • You should check for msgrcv() failing.

With these issues fixed, the corrected code looks like:

int pid;
int msqid;

msqid = msgget(IPC_PRIVATE, S_IRWXU);

if (msgid < 0) {
    perror("msgget");
    exit(1);
}

switch(pid=fork()) //fork child process
{//Child process
case 0:
    msg.mtype = 1;  /* Must be a positive value */
    strcpy(msg.mtext, "serving for sender\n");
    msgsnd(msqid, &msg, strlen(msg.mtext) + 1, 0);
    break;
case -1:
    printf("fork failed");
    exit(2);
    break;
default:
    wait(NULL);

    if (msgrcv(msqid, &msg, sizeof msg.mtext, 0, IPC_NOWAIT) >= 0)
        printf("%s",msg.mtext);
    else
        perror("msgrcv");

    msgctl(msqid, IPC_RMID, NULL);
    exit(0);
caf
  • 233,326
  • 40
  • 323
  • 462