In article <20060925042114.e94b7bc5.unex@linija.org>,
Mindaugas  <unex@linija.org> wrote:
>-=-=-=-=-=-
>
>Here is the updated patch, ready for criticism.
>
>I am not sure about an error code if shm_nused > newshmni, should be ok
>EPERM or better ECANCELED, or other?
>Is it safe to use the same shm_slock for locking in
>sysctl_ipc_shmmaxpgs()?
>Should the numbers in sysctl.h (see an XXX) be recounted?:)
>
>Thanks.
>
>-- 

+		shmmap_s = shmmap_getprivate(p);
+		/* Find our shared memory address by shmid */
+		SLIST_FOREACH(shmmap_se, &shmmap_s->entries, next) {
+			if (shmmap_se->shmid == shmid) {
+				if (cmd == SHM_LOCK)
+					state = FALSE;	/* Lock */
+				else
+					state = TRUE;	/* Unlock */
+				size = (shmseg->shm_segsz + PGOFSET) & ~PGOFSET;

I would re-write this as:

		state = cmd != SHM_LOCK; /* FALSE == Lock */
		SLIST_FOREACH(shmmap_se, &shmmap_s->entries, next) {
			if (shmmap_se->shmid != shmid)
				continue;
			size = (shmseg->shm_segsz + PGOFSET) & ~PGOFSET;
			....
		}

And of course decide what to do with the uvm stuff. The rest looks fine.

christos