> > > 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. > > > The proper fix is to escape shell metacharacters.
That is one (ugly, if you will) way to fix the problem that makes it impossible to open files with funny names. That, however, does not fix the segfaults, because if sh or ls fail for any reason, you'll again adjust the dot offset based on the char array which does not reflect reality. The metacharacter issue is also a bug not related to the warpping we're doing, and is probably best fixed in a separate diff. > There's already strspn(), why write your own > substitute for warpdot() ? Because llength() is the correct way to determine where a line ends in mg. Notably, lines may embed NUL bytes, and they're not required to terminate with one. Checking the length also makes sure you don't try to read an empty line (which may or may be a NULL pointer, I don't care). > You're also returning by supplying the function inside. > This makes the code less readable. I'm just returning (TRUE) because the function can never fail. You could also decide that it's appropriate to return (FALSE) if the 9th field cannot be located, but this doesn't change how the assignment should be done. The actual processing is done through pointers on the stack because we cannot do it via the global pointer (curwp), for the reasons I outlined in my previous message. > Lastly, the changes you made in dired_() seem > overcomplicated to me. If you prefer interleaving unrelated operations, be my guest. After all, it's not my choice. I just hope I needn't touch such code. > You're iterating twice for lforw() and d_warpdot(). If I wasn't already sick of this thread, I'd ask you to quote the exact lines of code to show where I'm iterating twice. Because I don't see it. > In the while() loop, we can already locate the .. and > we just need to lforw() there using the warp variable. > This seems redundant to me. Yes, we could, and I explained why I changed that (which is 1: giving warpdot only a pointer to char array is wrong, and 2: the code was interleaved with other things, namely the reading of ls output, and the closing of the pipe). There's a third reason, which is the misuse of strrchr to extract the filename. So I cleaned up the problematic bunch and fixed the segfault. > > > 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 > > It's a matter of style. The way you're designing the loops > and the functions look needlessly complicated to me, and > it's a bit hard to follow. God. Should forwchar() also return a dot offset and line pointer and let the caller assign them and decide how to handle the exception when they point out of bounds? What if this is a showstopper -- should the caller then return these two values on top of its own return value, and let the caller of the caller decide what to do? We might as propagate all errors to main(). Do you want that 20k line diff? > > 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). > > Warpdot() is meant to find the 9th column, if it can't, then something's > wrong. It should return -1 to indicate failure. To be safe, doto > should be set to 0. See above.
