Messages in this thread | | | From | Manfred <> | Subject | Re: [PATCH] 32-bit UID support for 2.3.30pre6 (important update) | Date | Mon, 6 Dec 1999 04:17:49 -0500 (EST) |
| |
> 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; > } > Yes.
> 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; No. A function should never release a lock. I though about the following code:
struct msqid64_ds tbuf; memset(&tbuf,0,sizeof(tbuf)); [...] msg_unlock(msqid); if (copy_msqid_to_user (buf, &tbuf, version)) return -EFAULT;
static void copy_msqid_to_user(void * buf, struct mgqid64_ds *msqid, int version) { msqid_ds old; if(version == 1) return copy_to_user(buf,msqid,sizeof(*msqid)); old.abc = msqid.abc; [...] return copy_to_user(buf,&old,sizeof(old)); }
The knowledge about the legacy support should be completely hidden within copy_xxx_to_user().
-- Manfred
- 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/
| |