OK, here's what I think.
1. if(childpid = fork()), should be if((childpid=fork()), this way the
boolean
gets tested for truness, not the assignment. Just the extra set of parens
is all
you need to be absolutely safe. I could be wrong, but it looks dangerous
to me.
2. You never set the initial value of the semaphore. You tried, by doing:
set_sembuf_struct(semwait, 0, -1, 0);
set_sembuf_struct(semsignal, 0, 1, 0);
But all this does, is set up your operation arrays. The actual value of
the
semaphore was still not set. Try this:
union semun {
int val;
struct semid_ds *buf;
ushort *array;
0 */
if(semctl(semid, 0, SETVAL, semu) < 0) {
/* handle error */
}
3. if ((childpid = fork()) <= 0), is bad because you are accidentally
checking for the instance of the child as
well as an error in the system call. I'm not sure why you were trying
to do... Try
if((childid=fork() < 0) {
/* error in system call */
}
else if(childid == 0) {
/* child does work here */
}
else {
/* parent does its work here or just breaks out of loop, or
whatever */
}
This way, you're cleanly testing all three results of fork(). Error,
child, parent.
4. Your first semop will wait for the semaphore to become 1, then it will
decrement the
value by one. But traditionally the first semop uses an instruction
array like this:
static struct sembuf db_lock[2] = {
0,0,0,
0,1,SEM_UNDO
};
semop(semid, db_lock, 2);
This tell the kernel to wait for semaphore # 1 to become 0, the
increment the value
by one.
Anyway, you've done stuff that doesn't seem to make alot of sense. Maybe
it's just a new
way of doing things, but I can't see how it would work. I think you main
problem it that you never initialized the semaphore. Try doing that first,
and then read Stevens "UNIX Network
Programming" There is a very detailed section on semaphores that you must
read over and
over again.
Frank Kolarek
fkola...@pol.net
> I am using some example code from my textbook for System V that uses
> semaphores to protect the critical section of a program that creates a
> chain of processes. I modified the code so that instead it would
> produce a fan of processes. The only line of code I changed was:
> FROM:
> if (childpid = fork())
> TO:
> if ((childpid = fork()) <= 0)
> But now when I run the executable with a command line argument for the
> number of processes I get the following error messages:
> ---------
> shell> a.out 3
> i:1 process ID:14661 parent ID:14660 child ID:0
> [14660]: semaphore decrement failed - [14662]:Identifier removed
> shell> semaphore decrement failed - Identifier removed
> shell> a.out 3
> i:1 process ID:14694 parent ID:14693 child ID:0
> [14695]: semaphore decrement failed - Invalid argument
> [14693]: semaphore decrement failed - Invalid argument
> --------
> I am not sure what is going on and was wondering if any of you can
> help me? The code is some what long so I hope you guys don't flame me.
> Here goes:
> #include <stdlib.h>
> #include <unistd.h>
> #include <stdio.h>
> #include <limits.h>
> #include <errno.h>
> #include <string.h>
> #include <sys/stat.h>
> #include <sys/wait.h>
> #include <sys/ipc.h>
> #include <sys/sem.h>
> #define PERMS S_IRUSR | S_IWUSR
> #define SET_SIZE 2
> void set_sembuf_struct(struct sembuf *s, int semnum,
> int semop, int semflg);
> int remove_semaphore(int semid);
> void main (int argc, char *argv[])
> {
> char buffer[MAX_CANON];
> char *c;
> int i;
> int n;
> pid_t childpid;
> int semid;
> int semop_ret;
> struct sembuf semwait[1];
> struct sembuf semsignal[1];
> int status;
> if ( (argc != 2) || ((n = atoi (argv[1])) <= 0) ) {
> fprintf (stderr, "Usage: %s number_of_processes\n", argv[0]);
> exit(1);
> }
> /* Create a semaphore containing a single element */
> if ((semid = semget(IPC_PRIVATE, SET_SIZE, PERMS)) == -1) {
> fprintf(stderr, "[%ld]: Could not access semaphore: %s\n",
> (long)getpid(), strerror(errno));
> exit(1);
> }
> /* Initialize the semaphore element to 1 */
> set_sembuf_struct(semwait, 0, -1, 0);
> set_sembuf_struct(semsignal, 0, 1, 0);
> if (semop(semid, semsignal, 1) == -1) {
> fprintf(stderr, "[%ld]: semaphore increment failed - %s\n",
> (long)getpid(), strerror(errno));
> if (remove_semaphore(semid) == -1)
> fprintf(stderr, "[%ld], could not delete semaphore - %s\n",
> (long)getpid(), strerror(errno));
> exit(1);
> }
> for (i = 1; i < n; ++i)
> if (childpid = fork())
> break;
> sprintf(buffer,
> "i:%d process ID:%ld parent ID:%ld child ID:%ld\n",
> i, (long)getpid(), (long)getppid(), (long)childpid);
> c = buffer;
> /************************entry section
> *************************/
> while(( (semop_ret = semop(semid, semwait, 1)) == -1) &&
> (errno == EINTR))
> ;
> if (semop_ret == -1)
> fprintf(stderr, "[%ld]: semaphore decrement failed - %s\n",
> (long)getpid(), strerror(errno));
> else {
> /****************start of critical section
> *********************/
> while (*c != '\0') {
> fputc(*c, stderr);
> c++;
> }
> fputc('\n', stderr);
> /******************end of critical section
> *********************/
> /*********************** exit section
> **************************/
> while(((semop_ret = semop(semid, semsignal, 1)) == -1) &&
> (errno == EINTR))
> ;
> if (semop_ret == -1)
> fprintf(stderr, "[%ld]: semaphore increment failed - %s\n",
> (long)getpid(), strerror(errno));
> }
> /******************** remainder section
> ************************/
> while((wait(&status) == -1) && (errno == EINTR))
> ;
> if (i == 1) /* the original process removes the semaphore */
> if (remove_semaphore(semid) == -1)
> fprintf(stderr, "[%ld], could not delete semaphore - %s\n",
> (long)getpid(), strerror(errno));
> exit(0);
> }
> void set_sembuf_struct(struct sembuf *s, int num, int op, int flg)
> {
> s->sem_num = (short) num;
> s->sem_op = op;
> s->sem_flg = flg;
> return;
> }
> int remove_semaphore(int semid)
> {
> return semctl(semid, 0, IPC_RMID);
> }