[crossfire] Crossfire server code cleanup/janitorial

[Kevin Zheng]

On 04/23/2014 02:17, Tolga Dalman wrote:
> It is not only the language and the employed toolchain. In my
> experience, problems begin as soon as you start implementing
> containers and data structures in C (which, btw, has been done in
> crossfire). It is then a good idea to use standardized library
> functions, like, strings, lists, or hash tables. All this is provided
> by C++ for free in a highly portable way.

While using C++ means that we can reinvent less wheels, the current
implementation is no less portable.

>> Right now the code seems to compile fairly cleanly.
> 
> Try compiling with gcc 4.8. While almost all issues are minor (unused
> variables and signed compares), there is some work left with at least
> GCC. I haven't tried other compilers though.

I'm using Clang and I haven't seen warnings about unused variables. I'll
poke around my compiler flags and see if I can get them.

>> While strlcat and strlcpy are not defined by any standards body, I 
>> believe that they are superior to the normal functions and should
>> be used whenever possible. If you haven't noticed, I recently
>> committed a small chunk of platform-dependent code that uses these
>> two functions when available instead of our hacks around it.
> 
> Yes, and that's a good progress! However, I still fail to understand 
> why strlcat/strlcpy would be superior to strncat/strncpy.

The standard "strncat" and "strncpy" functions do not guarantee that the
destination buffer will be null-terminated. This means that your changes
are vulnerable to many buffer overflow attacks.

Using the sstring construct in Crossfire or moving to C++ will mitigate
this possibility, but probably not completely remove it.

>> Even then, some specifics would be useful. What are you suggesting
>> for networking or the server loops?
> 
> I haven't had thought about it well enough, but I can think of two 
> possibilities: a) refactor/rewrite the code. b) use third-party
> libraries. My preference is of course b), however, such a step must
> be thoroughly investigated beforehand.

Some of the code in the network loop should be cleaned up. Once I was
also considering using something similar to SDL_Net, but decided that it
wasn't worth the effort.

> I won't be able to test any other platform than Linux and I won't be 
> attempting to do so either. If you think some of the changes are 
> worthwhile to adopt, I'd be happy of course. Likewise, please don't 
> hesitate to comment on my patches. I will try to keep the code in 
> sync, but at some point that will no longer be possible.

I noticed many changes to silence compiler warnings. I haven't taken a
closer look, but perhaps it may be a good idea to change the function
definitions themselves instead of prefixing "(void)"?

I also noticed a lot of changes that simply rip out compatibility in
'porting.c', which is intended to keep that kind of stuff. Ideally,
there would be no conditional compilations except there.

Extend the above mentioned issue with ripping out the safe_* functions.

I'll be working on cherry-picking specific changes, I hope your rebases
with master aren't too painful :)

Consider using `git svn` which lets you keep track of the SVN
repository. You can also rebase your work off my pre-cloned repository
and track SVN from there.

Thanks,
Kevin Zheng