> I think it's superflous to increase the length of the sequence number
> (ipc_perm.seq), it's limited to USHRT_MAX anyway [hard implementation
> limit, difficult to increase]
I just increased everything to machine word size with the thinking that
that would be optimal for integer math. (is it more efficient to avoid
shorts on risc architectures?)
> You have increased all limits to 'long', ie 64-bit on 64-bit platforms.
> Should we do that? If we increase them to 'long', then we should also
> modify the sysctls and the internal variables: most of them are 'int'
> values.
Thanks for pointing that out, I'll look at the sysctls and other
variables.
> This is required because certain limits are writable: what if root tries
> to increase the maximum message queue length to 0x1 0000 0000 L? I think
> these "if(value > INT_MAX)" lines are ugly.
I'll take a closer look at this. In the mean time, it shouldn't break
anything by switching to longs, even if values > INT_MAX can't be set,
right?
> And: What about making xxx64 a bit flag (eg 256)? This could clean up
> the code,
Hmm. Would you imagine doing something like:
asmlinkage long sys_msgctl (int msqid, int cmd, struct msqid_ds *buf)
{
...
int ipc_version = 0;
if(cmd & NEW_IPC64_FLAG) {
ipc_version = 1;
cmd ^= NEW_IPC64_FLAG;
}
...
If you'd prefer it this way I'll rewrite it.
> especially if you replace
> copy_to_user(src,dest,sizeof(msgid_ds)) with a special
> copy_msgid_id_to_user(src,dest,version)' function.
My only concern with that is that it would obfuscate the locking in the
code, specifically the dropping of the spinlock before doing a
copy_to_user:
if ( (cmd == MSG_STAT64) || (cmd == IPC_STAT64) ) {
struct msqid64_ds tbuf;
memset(&tbuf,0,sizeof(tbuf));
kernel_to_msqid64_ds(msq, &tbuf);
msg_unlock(msqid);
if (copy_to_user (buf, &tbuf, sizeof(tbuf)))
return -EFAULT;
How about rewriting the code as follows:
/* NOTE: copy_msqid_to_user does msg_unlock before the
actual copy_to_user, dropping the spinlock so we can
possibly sleep in copy_to_user */
if (copy_msq_to_user(msq, buf, cmd))
return -EFAULT;
static void copy_msq_to_user(struct mgq_queue *msq, void *buf, int cmd)
{
if((cmd == MSG_STAT64) || (cmd == MSG_STAT)) {
struct msqid64_ds tbuf;
memset(&tbuf, 0, sizeof(tbuf));
/* do copying of fields, etc. */
msg_unlock(msq);
return copy_to_user(buf, &tbuf, sizeof(tbuf));
} else {
struct msqid_ds tbuf_old;
...
}
}
Thanks for your help,
Chris Wing
wingc@engin.umich.edu
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/