[pulseaudio-discuss] [PATCH] shm: Don't force the shm files to be read-only

[Tanu Kaskinen]

On Tue, 2013-04-09 at 14:35 +0200, David Henningsson wrote:
> On 04/09/2013 10:32 AM, Tanu Kaskinen wrote:
> > Forcing the shm file to be read-only makes shm_unlink() fail on OS X.
> > Thanks to Albert Zeyer for finding the bug.
> >
> > BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=62988
> > ---
> >   src/pulsecore/shm.c |    3 ++-
> >   1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/pulsecore/shm.c b/src/pulsecore/shm.c
> > index 9f44df3..8aa34fa 100644
> > --- a/src/pulsecore/shm.c
> > +++ b/src/pulsecore/shm.c
> > @@ -111,6 +111,7 @@ int pa_shm_create_rw(pa_shm *m, size_t size, pa_bool_t shared, mode_t mode) {
> >       pa_assert(m);
> >       pa_assert(size > 0);
> >       pa_assert(size <= MAX_SHM_SIZE);
> > +    pa_assert(!(mode & ~0777));
> >       pa_assert(mode >= 0600);
> >
> >       /* Each time we create a new SHM area, let's first drop all stale
> > @@ -151,7 +152,7 @@ int pa_shm_create_rw(pa_shm *m, size_t size, pa_bool_t shared, mode_t mode) {
> >           pa_random(&m->id, sizeof(m->id));
> >           segment_name(fn, sizeof(fn), m->id);
> >
> > -        if ((fd = shm_open(fn, O_RDWR|O_CREAT|O_EXCL, mode & 0444)) < 0) {
> > +        if ((fd = shm_open(fn, O_RDWR|O_CREAT|O_EXCL, mode)) < 0) {
> >               pa_log("shm_open() failed: %s", pa_cstrerror(errno));
> >               goto fail;
> >           }
> >
> 
> Ack.
> 
> I would probably have done "mode & 0744" instead of skipping the mask 
> completely, but it's only called with the 0700 argument, so it doesn't 
> matter.

I think it would be strange to apply any mask here. I think the calling
code should be able to assume that pa_shm_create_rw() will use the
specified mode. If we want to force the mode to be stricter than 0777,
the assertion can be modified.

I'll apply this as is, changes to the assertion can be made later if
necessary. Thanks for the review!

-- 
Tanu