> The warpdot() has at least one issue. It leads to > segfaults if you try to open a directory like (BCD).
[.. diff ..] > ed > > mg doesn't segfault. Your fix is wrong, and only works by chance. If you craft a directory with enough spaces in its name, it'll segfault again. And you cannot fix this in warpdot, it's the wrong place. The bug is in dired_(), which should abort if the command fails. > Here's a snippet from ntpd.c > if ((pw = getpwnam(NTPD_USER)) == NULL) > errx(1, "unknown user %s", NTPD_USER); > When you're designing functions, you should account for > error values as well. -1 is used by most code written > from scratch in base. The ntpd quote is totally irrelevant. When designing functions, you have to account for errors, if such can happen. In library functions, returning an error value is often the only possible solution, because the library does not know how you want to solve the situation. In other places, there may or may not be a way to handle the error immediately. For a simple application, the developer could decide that it's simply not worth it trying to recover from a failed allocation; in this case, he handles it with a wrapper function which simply makes the program quit instantly. In other cases, you have options such as "do nothing", in which case the operation does not complete, but it doesn't cause a crash either. An example: trying to move the dot past the start or end of the buffer in mg would result in a no-op, and other functions do not have to try and verify that the dot is within the legal range. In the addline case, there is a way to handle the error: don't add a line if you can't allocate it. The error is handled in such a way that most of the other functions do not have to worry about it. There are other places where errors are handled "up the stream" likewise. If you still think this is wrong, I could cook you a 20k line diff that makes every single function check that 1) the character we read from stdin isn't EOF, 2) curwp is not null, 3) curwp->w_dotp is not null, 4) curbp is not null, and so on. By now you should realize how ridiculous this is. These errors are (or should be) accounted for up the stream so that none of the functions that actually have to operate on existing buffer or window contents should worry about it. > Your warpdot() works differently > and doesn't quite conform to style. You're assigning > a value to a variable without checking if this is correct > or not. This style is hard to read according to me. Bullshit. It's not a matter of style; it's a question of whether or not an error can happen, and what to do when it happens. In this case, if there was an error to handle, we could return zero, which will always be a legal value (since we are assigning to an int which is zero if the line is empty). If warpdot was a library function whose callers need to react to the error in different ways, then we'd have to return an error value. Like I said above, the segfault you got has nothing to do with warpdot, and trying to fix it there is wrong.
