Richard Hansen wrote:
> Greg Hudson wrote:
>> * This is a simple wrapper program, but it has been coded as a
>> capital-p-Project. It has five source files and 1939 lines of code
>> (mostly comments), separate src/ and include/ directories, and man
>> pages for the internal functions contained within. The ratio of
>> overhead to actual functionality is off the chart, especially for a
>> program which ought to be easy to audit (since it runs with elevated
>> privileges).
>
> True, there is a lot of non-code. I would expect that to be a welcome
> thing; most people complain about the *lack* of documentation. I
> figured that the extensive documentation would make it easier to audit,
> not harder.
Documentation is good, but I'd have to agree with Greg here about the
ratio of project infrastructure to actual functionality here being
ludicrous :-)
>
> sloccount gives me 846 lines of code, and 300+ of that is for some
> aggressive safety checks to help ensure the user can't exec() an
> arbitrary executable and gain privileges. (Note that the wrapper should
> NOT be installed setuid/setgid root -- there should be an unprivileged
> user/group specifically created for accessing the repository. This way,
> if a user does find a vulnerability in svnstsw or svnserve, the user
> would gain full read/write access to the repository and nothing else.)
>
Is it really the intention that any of the C APIs be consumed outside
the program? If so, is that intention realistically sensible?
My instincts here would be to abandon building a "libsvnstsw", remove
the installed header files and the section 3 manpages, and install just
one executable binary and its section 8 manpage.
>> It would be disappointing to see stuff added to the project's main
>> autoconf grot in order to support a contrib program.
>
> I expect most developers feel this way, but I'm not sure so I thought
> I'd ask. Most of the autoconf stuff currently in svnstsw can be left
> out without major loss of functionality, with C99 being the exception.
> It would make my life easier if I could test for C99; if the community
> has substantial reservations about it, I'll have to move to plan B.
I don't actually mind a moderate about of feature tests for a contrib
program, provided they aren't ones which take a long time to run.
However, I hold the opinion that we build contrib programs from the main
buildsystem because contrib programs are often tiny little programs
which don't have a buildsystem of their own. That clearly isn't the case
here, and IMO, if svnstsw is going to maintain the rest of its
Project-with-a-capital-P nature, it might as well keep its own
buildsystem too - indeed, to not do so would be bizarre.
> If conditional compilation is not possible with Subversion's current
> build system, or if testing for C99 functionality is out of the
> question, my options are: (1) remove svnstsw entirely, (2) add the
> ability to conditionally compile code to Subversion's build system, (3)
> re-write svnstsw without using C99 features, or (4) leave everything how
> it is right now.
>
> It would be sad to choose option #1 since I think that this code is
> useful to some people. I'm reluctant to pursue option #2 for just a
> contrib program since it would be a major change to the build system.
> Option #3 would take time, and would require introducing malloc() into
> the code. Option #4 is easiest, but it makes life harder for
> users/package maintainers, plus developers have already expressed
> discomfort with this option.
>
> There is another option: add the equivalent functionality to svnserve
> directly. There would be some backwards-compatibility concerns, however.
Proper support for conditional build targets would be valuable for other
reasons - making the conditional building of RA/FS libraries depending
on the presence of BDB, Neon and Serf nicer - but would be a lot of work.
I think what to do here largely depends on what the eventual intent for
this code is - I think it should either work towards being integrated as
a part of Subversion itself, be it either as a part of svnserve or as a
separate executable, or it should spin off into a separate project
entirely, with its own releases, repository, and all that.
Max.
Received on 2008-05-06 23:17:33 CEST