Subject: Re: Dynamic limits for SysV semaphores and message queues
To: None <firstname.lastname@example.org>
From: Mindaugas R. <rmind@NetBSD.org>
Date: 10/07/2007 16:48:59
Greg Troxel <email@example.com> wrote:
> You have hoisted the special case out of the loops, and I think in one
> case this introduced a bug: msghdrs[msginfo.msgtql-1].msg_type doesn't
> get set to 0. I'd prefer to leave the loops whole and use the if in
> these cases, as it's clearer to verify correctness. I think your change
> to make the top element the special case one helps understandability.
Actually, it does not matter since the memory is zeroed. But yes, I will set
msg_type for the readability. However, moving such special cases out of the
loop where appropriate makes it more readable for me.
> EPERM for not the new memory being smaller seems like the wrong code,
> because the user might well have permission, but the kernel is enforcing
> an invariant even on root.
Right, EBUSY or EAGAIN would be a correct value. I will take EBUSY.
> Does the new_msgpool = ptr + size idiom adequately ensure alignment? If
> it does because of the structure contents, a comment explaining why
> would be nice to perhaps avoid future problems. Or a KASSERT about
> size/alignment. (Did you try this on sparc64?)
I am not sure what do you mean. Do you mean a correct counting of offsets?
In such case - yes. Also, take a look at msginit() or seminit().
Re sparc64: No. Volunteer to test? :)
> Again on realloc "new_msghdrs[i].msg_type = 0;" does not get executed
> for the highest new_msghdrs[i].
I will change this for the readability.
> I don't see how the if(msg_realloc_state) (at 455/659) interacts with
> reallocation, because it looks like msgmutex takes care of this.
In case there are waiters, msgrealloc() might sleep in cv_wait(). At that
moment, another msgrcv() call might happen which waits on cv_wait_sig().
This check disallows such situation.