On Fri, May 21, 2004 at 04:39:02AM EDT, Juergen Salk wrote: > David Yitzchak Cohen wrote: > >> mutt complains that "~/.mutt/scripts/chkmbox|" does not exist. > > > The bug was fixed in CVS only a few days after 1.5.6i was > > released. > > Oh, I see. I was already wondering why I couldn't find any bug > reports addressing this issue. Yeah, a bug report for the original issue was never actually filed. > This is a real showstopper, > IMHO. SUSE 9.1 ships this broken version, btw. :-( Yeah, maybe you should tell them to revert to 1.5.5.1i (which has backwards-compatible alternates, subscribe, and lists statements, anyway), or update to a relatively recent CVS version? > > In other words, no stat(2)ing should be done at all if it's a > > command. The patch in CVS does just that. > > It seems, this one just reverts another patch that has introduced > the bug earlier, right? Well, this patch redid the previous patch to (a) do checking in the right place; and (b) not attempt checks that only a shell is capable of doing right. (Picture the classic case of a shell builtin that's not also available as an external binary, or an alias, or a function, for example. Mutt would have no way of knowing that the shell _can_ actually execute the thing. Also, the vanilla stat(2) done by the code doesn't check whether or not the thing's executable to the user at hand, or that the particular shell in use allows execution of that particular file (even if it is executable according to UNIX permissions), so there's no general way to cover the other direction either. In other words, there's little to be gained by having Mutt preemptively attempt to verify whether or not a command is valid.) > >> For what it's worth, here is a patch that seemed to fix the > >> problem for me. > > > Well, it fixes your particular case, but leaves many other cases > > broken. > > Most notably command line options. True. Well, yeah, even for a command that actually _is_ (also) an external binary, Mutt's current checking code isn't smart enough to toss everything after the first space (for a Bourne-compatible or csh-compatible shell, that is; for a shell such as CINT for instance, spaces don't indicate argument separation ... of course, there are good reasons why you wouldn't want to symlink CINT to /bin/sh, anyway). > > These checks are better done in mutt_open_read(), which you're > > about to call anyway: > > Yup. The CVS version seems to do this in mutt_open_read(). mutt_open_read() is the function that actually does all the gruntwork here. It's gotta do a bunch of checks anyway, and it's in a position to do almost all checks. You might as well let it handle it all. It also has the neat advantage that if you ever want to switch from trailing pipe notation to leading bang notation (which avoids the need for a strdup(3)) or some other notation, you only have to change the mutt_open_read() function. > However, 1.5.6i does it in source_rc(). That's because 1.5.6i was released a little after the first patch, but just before the second patch. In retrospect, releasing a 1.5.6.1i probably would've been a wise idea. I don't think anybody had any idea back then, though, about how many people would actually complain (i.e., about how many people actually use the piping feature for scripts) about 1.5.6i. > So I tried to fix it in > place, which was obviously a waste of time. :-/ You learned a fair amount about the Mutt code in that area. I don't consider that a waste of time :-) > > Thanks for taking the initiative to fix bugs, rather than simply > > reverting back to 1.5.5.1i :-) > > Well, I deny myself telling you which version I came from. ;-) 1.2.something? Take care, - Dave -- Uncle Cosmo, why do they call this a word processor? It's simple, Skyler. You've seen what food processors do to food, right? Please visit this link: http://rotter.net/israel
Attachment:
pgp0QFI07oGNM.pgp
Description: PGP signature