[pptp-server] proposed changes to sources

[David Luyer]

> After starting to read the source, to try to understand what it is
> actually *doign* (as opposed to compile-run-debug-etc.) I noticed
> that pptpd.c and in turn pptpd doesn't do much except daemononize
> itself and then exec pptpmanager.

I like the current model, and the reason for it has been explained by Matt R;
for uClinux where fork() is not available (must vfork()).  Also, keeping
pptpd and pptpmanager separate means that you have a smaller binary actually
being executed, the pptpd bit is thrown away on startup.

> I propose to merge these together, so that there are only two
> binaries pptpd and pptpctrl. Eventually, we should be able to merge
> these two into one binary that just internally forks and calls a
> control session instead of needing another binary.

As above.  If we want to merge into one binary, the squid model would be
better.  See also my direct mail to you about resource limits bottlenecks.
I'm not sure about the non-Linux limits for all of this though.

fork() without exec() tends to not be a good idea as it makes programs
more confusing (eg, variables which become out of date but remain accessible)
without a real gain, when there are so many other models to choose from.
It can also be horribly inefficient (keeping a copy of unused parts of the
dataset around as at each fork()).

> Ultimately (sp!) we
> could view having the entire thing threaded [...]

Not sure how well this would work, and whether the locking overheads and
code complexity would be worth it.  It is worth consideration, but I'd
prefer the single process model (even with the file descriptors issue
it would introduce; threading would introduce the same issue but not
achieve the same performance).

> Meanwhile, unless someone can explain why pptpd.c exists as a program
> and not just a daemon() function, I will go ahead and merge them.

See Matt R's e-mail/my attempt to explain above.

> BSD has a daemon() function that I would like to use if available, and
> I will provide a slightly modified version of the code in pptpd.c to
> simulate it where not available.

Linux should definitely be using setsid() as it presently does.

> I think it is likely that we are goign to need a compat/ directory for
> code which is OS dependent, and build that into a .a file which is
> likned against the main program - mainly to help us in debugging and
> to stop code pollution. Thoughts ?

I think that's excessive at present - the code is relatively small and
generally tries to be POSIX.  But if we start building a lot of extra
bits for other operating systems, it's probably worth it.  For now a
simple 'compat.c' with the relevant stuff in it would probably do it.

> Almost done, but who knows about the EMBED code and its uses ?

It's because the pptpd was originally written for the coldfire uClinux
port, which, for example, doesn't have a fork() call.

> BTW - last one - can someone who knows the GNU libs better pull in a
> version of getopt_long() for just this purpose, since BSD's don't
> carry one :(

You could install libiberty and link with -liberty, to be freed of your
OS's restrictions :-)

I'll try to merge in a getopt_long() today.  If pptpd gets demand on other
OS's, we'll also need things like inet_ntoa(), strdup(), etc, which aren't
fully portable to be built.  I guess these do probably belong in a compat
library.

Hmm... how does pptpd go under cygwin32?  As an option for people who don't
want to pay for a client access license for every VPN user on NT.

David.