Path: utzoo!attcan!uunet!lll-winken!ames!xanth!ukma!cwjcc!hal!ncoast!allbery From: allbery@ncoast.UUCP (Brandon S. Allbery) Newsgroups: comp.sources.d Subject: Re: Comments on INSERT.c (was: Re: It is better to remain silent...) Message-ID: <13336@ncoast.UUCP> Date: 14 Jan 89 20:52:23 GMT References: <308@twwells.uucp> Reply-To: allbery@ncoast.UUCP (Brandon S. Allbery) Followup-To: comp.sources.d Organization: Cleveland Public Access UN*X, Cleveland, Oh Lines: 112 Bill's not going to be too happy with me: I side with Bob Webber on this. [Heresy! ;-) ] As quoted from by webber@athos.rutgers.edu (Bob Webber): +--------------- | In article <308@twwells.uucp>, bill@twwells.uucp (T. William Wells) writes: | > First, let's notice that this is essentially equivalent the cp | > command, minus cp's frills. It does do one thing that I don't believe | > all cp's do: it verifies that the copy succeeded. But one can do the | > job with a little shell script that does a cp, some ls's, and some | > tests. Thus this program is an example of bad analysis: it is a new | > program to do what existing tools can already do. | | No, it is fundamentally different from cp. cp truncates to 0 an | existing file (or unlinks it) before writing into it (or creating | a new one). INSERT writes into the file and then truncates it to | the appropriate size. This difference is crucial if you are on a | multiuser system that is out of file space. +--------------- One could argue that the difference should not be necessary. But I've seen cases where it would have been quite nice. +--------------- | > Second, it is nonportable. It is Berkeleyoid specific code. | > Ftruncate marks it as such. There are also those UNIX-specific I/O | > calls. | | It is as portable as possible. Tell me how to accomplish the same | correctly on a POSIX/v7 system and I will be happy to make the program | more portable. [By the way, a request for such an equiv has been | posted to comp.unix.wizards, so people interested in this coding | aspect should watch for conversation on it in that group (assuming | that there is anything to say other than it just can't be done except in BSD).] +--------------- Xenix 5.3.1, at least, has chsize(); this amounts to the same thing. V7 and System V.2 have no such functionality. I don't know about Posix. Bill: ftruncate() being the whole point of the program, referring to it as a non-portable cp sounds like a missed target to me. +--------------- | > Third, it is a bad program, even if we didn't care about portability. | > | > 2) The lack of variable naming consistency. If we're going to | > have the verbosity of words-for-names and capitalization of | > each word, names like `fdin', `fdout', and `chs' don't belong. | > (This one is arguable.) | | Consider it argued. fdin, fdout, and ch are sufficiently standard in | unix programming that they are almost predeclared. chs for multiple | ch's is just a bit of humor. The rest of the names are equally | descriptive, although they are longer, e.g., ReadExit and ReadSize | (perhaps ReadTotal would be even better, but I will hold off a release | for that until some more useful can be bundled with it). +--------------- I have yet to be flamed for calling a random counter variable "cnt", a random stat buffer "sb", a random file descriptor or file pointer "fd" or "fp" (respectively), a random char pointer "cp", etc. This is a foolish attack, given that you haven't flamed anyone *else* who uses the convention. +--------------- | > 3) The unnecessary code sequences. The lseek's and the ftruncate | > serve no purpose that I can see. | | The lseeks position the file at the beginning. Oddly enough, the man | page on open doesn't assert that this is default on our system. From a +--------------- UNIX seems to assure it; but for portability, you hit the mark. I've *used* OSes where the file pointer was undefined after an open(). [I prefer not to think about them...!] Unfortunately, the use of the low-level calls -- including ftruncate() pretty much invalidates the portability. Nice try, though. [Actually, enough C libraries implement open(), read(), write(), etc. on non-UNIX systems that they can be considered at least somewhat portable.] +--------------- | > 4) The use of a cast to create a long constant. This indicates | > ignorance of C, or a slavish obedience to a not-understood | > coding standard. | | Aren't you worried about portability to systems with 16 bit ints | running BSD? The man page clearly says that arg is a long and lint | expects it. +--------------- Complaining about the use of "(long) 0" when "0L" will do is along the lines of complaining about the use of "ls ." when "echo *" will do.... +--------------- | > We'll further note that writing a structured program and using a | > consistent style (except for the naming and one minor glitch) did not | > prevent him from writing a bad program. | | That remains to be demonstrated. +--------------- It also raises a *true* point: the use of structure and consistent style didn't make the program's *purpose* clear to at least one net.reader. Which is why "self-documenting" languages and programming styles have always failed. ++Brandon (P.S. Bill: Take a Valium and try again when you're calmer.) -- Brandon S. Allbery, moderator of comp.sources.misc allbery@ncoast.org (soon) uunet!hal.cwru.edu!ncoast!allbery ncoast!allbery@hal.cwru.edu Send comp.sources.misc submissions to comp-sources-misc@ NCoast Public Access UN*X - (216) 781-6201, 300/1200/2400 baud, login: makeuser