Date: Sun, 12 Jan 2003 19:39:56 +1100 (EST) From: Bruce Evans <bde@zeta.org.au> To: Jake Burkholder <jake@locore.ca> Cc: Matthew Dillon <dillon@apollo.backplane.com>, <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 Message-ID: <20030112191531.J5936-100000@gamplex.bde.org> In-Reply-To: <20030112014644.F212@locore.ca>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, 12 Jan 2003, Jake Burkholder wrote: > Apparently, On Sat, Jan 11, 2003 at 06:54:53PM -0800, > Matthew Dillon said words to the effect of; > > > ... > > :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. > > > > Well, a good bit of the casts removed also allowed the parenthesis to > > be removed, a couple of split lines could be put back together, and > > generally it has made it more readable. > > Exactly 1 line unless I completely failed in generating an accurate diff > of your commit. The lines would be shorter if you had just removed the > unnecessary casts. In the only .c file that I've looked at so far (kern_acl.c), there is 1 completely unsplit line and 3 missed opportunities to (partially) unsplit lines, leaving the latter more misformatted than before. The changes in file.h have larger style bugs. > I find the code more readable with just the unnecessary casts removed. I agree. Removing them has been on my todo list for many years. > > ... > > :I'm really baffled as to why you did this. > > > > It's a cleanup. Ugliness I noticed as I was evaluating how easy it > > would be to add a memory object file type. I don't see why you believe > > a justification is required other then cleanup and code readability. > > void *'s are things you only use when you need to. We certainly didn't > > need to here. How exactly is this commit so bad that you are against it? > > Its unnecessary code churn. I wouldn't be surprised if struct file was > used all the place in 3rd party modules, especially MAC modules. Now > they either need an ugly compat define or ugly idfefs. Some of your commits > were to code that's externally maintained. We've broken compatibility with > external code before for compelling reasons. This is not a compelling reason. > > You obviously don't think this is important, I do. If no one else cares about > this I'll shut up now. I agree with all your points. I slightly prefer "void *" to the union, but wouldn't change either once it is in use. Bruce 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?20030112191531.J5936-100000>