[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH,HURD] Add SysV SHM support
From: |
Kalle Olavi Niemitalo |
Subject: |
Re: [PATCH,HURD] Add SysV SHM support |
Date: |
Mon, 14 May 2012 20:16:39 +0300 |
User-agent: |
Gnus/5.110007 (No Gnus v0.7) Emacs/23.0.51 (gnu/linux) |
Samuel Thibault <samuel.thibault@gnu.org> writes:
> +/* Removes a segment attachment. Returns its size if found, or EINVAL
> + otherwise. */
> +error_t
> +__sysvshm_remove (void *addr, size_t *size)
Doesn't literally return the size. There is a more accurate
comment in sysvshm.h; perhaps this one could be just removed.
In that function:
> + if (shm->addr == addr)
> + {
> + *pshm = shm->next;
> + *size = shm->size;
> + __mutex_unlock (&sysvshm_lock);
> + return 0;
> + }
Shouldn't this free (shm)? It seems to leak otherwise.
In sysdeps/mach/hurd/shmat.c (__shmat):
> + res = __fstat (fd, &statbuf);
> + if (res < 0)
> + {
> + __close (fd);
> + return (void *) -1;
> + }
Why doesn't this save and restore errno across __close like
__shmctl does? I expect it's necessary in both or in neither.
In sysdeps/mach/hurd/shmctl.c (__shmctl):
> + buf->shm_perm.__key = id;
If id refers to a shared-memory segment created with key ==
IPC_PRIVATE, then shmctl IPC_STAT should set buf->shm_perm.__key
= IPC_PRIVATE. At least, that's what happens under GNU/Linux.
It seems this would be easy to implement in __shmctl.
In sysdeps/mach/hurd/shmdt.c (__shmdt):
> + __munmap ((void *) shmaddr, size);
> + return 0;
I wonder if this should check for errors from __munmap.
In sysdeps/mach/hurd/shmget.c (get_exclusive):
> + /* Try to link the shared memory segment into the filesystem
> + (exclusively). Private segments have negative keys. */
They don't have negative keys. Both SHM_PRIV_KEY_START and
SHM_PRIV_KEY_END are positive.
pgp6R9S48EUOi.pgp
Description: PGP signature