Date: Sat, 11 Jan 2003 21:32:59 -0500 From: Jake Burkholder <jake@locore.ca> To: Matt Dillon <dillon@FreeBSD.org> Cc: cvs-committers@FreeBSD.org, cvs-all@FreeBSD.org Subject: Re: cvs commit: src/sys/kern kern_acl.c kern_descrip.c kern_event.c kern_mac.c sys_pipe.c sys_socket.c uipc_socket.c uipc_syscalls.c uipc_usrreq.c vfs_aio.c vfs_syscalls.c vfs_vnops.c src/sys/netgraph ng_socket.c src/sys/sys file.h src/sys/ufs/ffs ... Message-ID: <20030111213259.E212@locore.ca> In-Reply-To: <200301120137.h0C1bD0E098037@repoman.freebsd.org>; from dillon@FreeBSD.org on Sat, Jan 11, 2003 at 05:37:13PM -0800 References: <200301120137.h0C1bD0E098037@repoman.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Apparently, On Sat, Jan 11, 2003 at 05:37:13PM -0800, Matt Dillon said words to the effect of; > dillon 2003/01/11 17:37:13 PST > > Modified files: > sys/kern kern_acl.c kern_descrip.c kern_event.c > kern_mac.c sys_pipe.c sys_socket.c > uipc_socket.c uipc_syscalls.c > uipc_usrreq.c vfs_aio.c vfs_syscalls.c > vfs_vnops.c > sys/netgraph ng_socket.c > sys/sys file.h > sys/ufs/ffs ffs_alloc.c > sys/vm vm_mmap.c > usr.bin/fstat fstat.c > usr.bin/sockstat sockstat.c > usr.sbin/pstat pstat.c > sys/alpha/osf1 osf1_mount.c > sys/compat/linux linux_file.c linux_stats.c > sys/compat/svr4 svr4_fcntl.c svr4_filio.c svr4_ioctl.c > svr4_misc.c svr4_socket.c svr4_stream.c > sys/dev/streams streams.c > sys/fs/fdescfs fdesc_vnops.c > sys/fs/fifofs fifo_vnops.c > sys/fs/portalfs portal_vfsops.c portal_vnops.c > sys/fs/unionfs union_subr.c > sys/i386/ibcs2 ibcs2_misc.c ibcs2_stat.c > sys/netsmb smb_dev.c > sys/nfsserver nfs_syscalls.c > sys/opencrypto cryptodev.c > contrib/ipfilter/ipsend sock.c > Log: > Change struct file f_data to un_data, a union of the correct struct > pointer types, and remove a huge number of casts from code using it. > > Change struct xfile xf_data to xun_data (ABI is still compatible). > > If we need to add a #define for f_data and xf_data we can, but I don't > think it will be necessary. There are no operational changes in this > commit. Is removing the casts the only justification for this commit? void * can be assigned to any pointer type without a cast, so I really don't understand why this change was necessary. The only way that a cast is actually needed is in cases like this: ((struct vnode *)fp->f_data)->v_mount. In the majority of cases that I looked at the pointer is assigned to a local variable of the correct type first, in which case the cast is almost certainly a historical remnant of f_data having type caddr_t. Removing the unnecessary casts in that case I agree with, but changing the type of f_data as well I don't agree with at all. This is also a layering violation. The file layer is not supposed to know what the underlying object is. I understand that the generic pointer deals with that, but I don't see how f_data was insufficient and I think its wrong to specify in anyway other than void * what the underlying object is. I'm not at all happy with this change and I would like some justification for it other than to remove casts; in the majority of cases they are unnecessary and should have been just removed, and the remaining cases would have been better served by first assinging to a local variable of the correct type. I'm really baffled as to why you did this. Jake To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe cvs-all" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20030111213259.E212>