Date: Wed, 7 Feb 1996 03:42:29 -0800 From: Julian Elischer <julian@ref.tfs.com> To: hackers@freebsd.org Cc: terry@freebsd.org Subject: corrected context diffs of terries patches Message-ID: <199602071142.DAA00568@ref.tfs.com>
next in thread | raw e-mail | index | archive | help
I've been through his patches. I would really like someone ELSE so go through them too. I didn't see any problems, and I did like the restructuring of who frees what with regards to the namei name buffer it was definitly broken before.. I've placed a file in ftp::/freefall.cdrom.com:/incoming/context-diffs-terry with a slightly fixed up version of his diffs. (fixed a spammed file) comments. The first HALF (actually 60%) is MOSTLY cosmetic changes terry re-wrote almost all of vfs_syscalls.c so that all functions have a single exit point. My guess is that he plans to be able to do somethig at this exit point at some point in the future. (?) I am in two minds about this.. 1/ I think it's slightly cleaner code 2/ it might be a few cycles slower in places 3/ It makes it harder to merge in 4.4lite2 patches 4/ It's not ALWAYS easier to read, and is often harder to read because you have to find the end of the nested if'else'if sets to figure out where the code goes, while 'return EINVAL;' is pretty bloody obvious. 5/ It's easier to have leaks of things with many exit points 6/ it obfuscates any REAL changes he's made in the area and might intruduce new bugs for little gain. I'm kinda against these cosmetic patches.. unless terry can give a good reason. Terry, if you could extract the REAL changes from this stuff and submit that I'd be pretty happy with the patch. the last 40% of the context diff is real changes to do with the re-assigning of responsibility for freeing the name buffer in the namei code. This is a good idea, though I haven't checked 100% of the change yet. I think this part should be applied.. Any one Else care to look at it and comment? especially on the last 40%? If I can get terry to undo the cosmetic changes and submit them totally separatly the first 60% of the patch would shrink to about 10%.. julian
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199602071142.DAA00568>