jkamens at Advent
May 13, 2009, 9:33 AM
Post #1 of 6
Thanks for the feedback. Please forgive the formatting of this response; I am unfortunately stuck using Outlook :-/, and I'm sure it's going to stick line breaks in the wrong places.
Re: PATCH: Enhanced exclude / include support for clamscan
>While these features might make a lot of sense, I can't help thinking
>that this isn't the 'Unix' (right:) way to go about implementing them.
I've been using Unix for over two decades and have been known to edit raw sendmail CF files, so I think I know a thing or two about the "Unix way" :-).
On the one hand, you're right that were clamscan to have a command-line option to tell it to read a list of files on stdin, the logic I built into it could be implemented externally, either with home-grown scripts or with a separate utility bundled with clamav. I certainly wouldn't be adverse to achieving my goal that way, where that functionality to be provided by clamscan.
On the other hand, I don't quite view clamscan as being a pipeline tool of the same genre as sed, tr, etc. I think of it as a full-fledged application, and as such, I think it's reasonable for this kind of functionality to be built into it if it's something that a lot of people are going to find useful.
Other applications in the same genre with similar functionality include rsync, rdiff-backup, and GNU tar. All of those, like clamscan, could force the user to implement exclude / include functionality externally and then feed a file list into the application, and yet all of them provide complex, flexible built-in filtering functionality of their own.
Perhaps what is needed is a compromise -- basic filtering of the sort I implemented, *plus* the "-f" flag (or the equivalent) for people who want to roll their own logic?
It is also worth noting that I was not adding new functionality to the application but rather enhancing existing functionality. If there had been no --exclude and --include flags in clamscan to begin with, I probably would have implemented something like what you propose. But given that they were there and buggy (recompiling all the regexps for every file / directory being considered), I felt that fixing / enhancing the existing functionality was the right way to go.
>You mentioned maintenance. IMO, what you're doing is asking for a
>maintenance headache. You mentioned changing the API. Please, for
>the sake of people that are using the package, already, don't do that
>unless there is (a) a VERY compelling reason and (b) NO other way.
Interfaces change. Such is life. Remaining wedded to broken interfaces leads over time to difficult to maintain code with inferior functionality and performance. The changes should be visibly documented, and people who are using ClamAV should be smart enough to read the release notes when they upgrade.
If GNU tar, a far more widely used utility than ClamAV, can get away with changing its interface as frequently as it does, then I can't get too worked up over the relatively minor change I've proposed to the ClamAV interface.
Having said that, if the maintainers think that this particular interface change is a bigger deal than I think it is, then there are alternatives, e.g., adding a flag to enable the new exclude / include behavior and emulating the old behavior if that flag isn't specified. Or changing the names of the options for the new behavior (although I cringe a bit at the thought of that, since "--exclude" and "--include" are the most obvious and intuitive names for the options).
>I'm already leaning towards abandoning ClamAV because of instability
>in the API. Sanesecurity is the only reason I continue to use it.
It would seem, then, that the maintainers of ClamAV may share my philosophy about interface changes :-).
Please submit your patches to our Bugzilla: http://bugs.clamav.net