Greg Troxel <gdt@ir.bbn.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.

-- 
Best regards,
Mindaugas
www.NetBSD.org