Complexity of reallocating storage (was users command crap)

Flint Pellett flint at gistdev.gist.com
Wed Feb 6 03:59:08 AEST 1991


brnstnd at kramden.acf.nyu.edu (Dan Bernstein) writes:

>Summary: Chip complains that a read() from disk into a ``newuid''
>integer may fail and hence have pty switch to the wrong uid. But I
>explicitly initialized newuid to uid just to handle this possibility.
>Can people stop taking my code out of context?
...
>So, Chip, all that will happen upon an I/O error in that section is that
>pty will continue with the same uid as it had before. If communication
>(or the disk) breaks down entirely then the signaller process will lose
>track of the master process, but it is practically impossible that the
>section of code you quoted will result in a wrong uid, even in the
>already disastrous situation that there are physical disk errors. You
>say ``it will result in a pty program continuing to run using wrong
>data''; I say that you don't know what you're talking about.

>I put that newuid = uid initialization in for a reason...

I haven't followed this entire thread, but I think I've seen enough of
it to make a couple general observations that may or may not apply
directly here, which other people might be able to learn from.

1. If you post any code on usenet, expect it to be criticized.  If you
can't handle lots of criticism, don't post the code.

2. It would be nice, if people criticizing code did so as if were a
professional code review.  In the first place, remember that in a code
review it is the code that is being criticized, NOT the programmer. 
There is a difference between someone "being wrong on this point" and
"not knowing what they are talking about".  If you don't have any
respect for the person whose code you are criticizing, my suggestion
is to not bother doing it: you aren't going to help them or yourself. 

3. Regarding the code: I didn't see the entire example code, but from
the ensuing discussion, I conclude that there was a problem in the
comments in the code.  It was stated that the initialization was put
in "for a reason".  If the comments were complete, that would have
been obvious, and in fact that whole paragraph that appears above
explaining what the reasons were, and the reasons why the error
conditions are being ignored, should have been there as a comment,
even if the result was 15 lines of comment for 1 line of code.
When code is commented like that, then a maintainer who comes
along trying to get/keep the code working in a slightly different
environment can quickly determine if the "reason" for the check is
still valid, instead of having to spend who knows how much time trying
to figure out what the reason for it was.  

4. A good maintainer (or criticizer) always assumes the original
author did things for reasons.  If you assume that the original
author was just stupid and didn't know what they were talking about,
it may save you the time of figuring out what the reason for having
a check was, making it ok to remove it or alter it.  Then you
"test" it, it works ok, and you can go on your way, leaving a
time-bomb sitting around waiting to blow up when the reason for the
test you screwed up becomes apparent.) 
-- 
Flint Pellett, Global Information Systems Technology, Inc.
1800 Woodfield Drive, Savoy, IL  61874     (217) 352-1165
uunet!gistdev!flint or flint at gistdev.gist.com



More information about the Comp.bugs.4bsd.ucb-fixes mailing list