Re: [crossfire] Crossfire server code cleanup/janitorial

[Mark Wedel]

Few general notes on this:

My idea if using new functions provided by libraries (if they exist) and include 
local copies if they don't is that often times vendor provided functions may be 
much better optimized that locally included ones.


 Also, for some functions (like strlcpy), it may be a trivial exercise to 
include a local copy of equivalent functionality.  But for others, like 
snprintf, it might be hard, but a simple replacement of non equivalent 
functionality is easy, like:


sprintf(buf, ...)
if strlen(buf)  buflen { oops, we have a buffer overrun }

 which isn't great, but could still be better than an alternative of just using 
an unprotected sprintf.


 I do tend to agree with the comment that given crossfire is C code, using a C 
compiler would seem to be the best tool to use to compile it.  That said, if it 
happens to compile with a C++ compiler, or changes are trivial to let it do so, 
great.  But I'd just put the responsibility of making sure it compile with C++ 
belongs to those who want to compile with C++, not all developers.


 I will also note that crossfire actually does a fair amount of floating point 
- all the speed and speed_left is floating point, the just simple 
addition/subtraction.


 The plugin logic has pluses and minuses - the ability to right plugins that 
register themselves is good, but the entire dynamic loading adds a bit of code 
complexity and another place for errors.  But also, at one time, the python 
plugin was optional, so you could run the server without necessarily having 
python  its development libraries installed.  But IIRC, at some point, it was 
decided that enough scripts and other stuff require python it should become 
standard - since it is known it will always exist, whether having it be a 
dynamically library that is loaded makes sense is debatable.




___
crossfire mailing list
crossfire@metalforge.org
http://mailman.metalforge.org/mailman/listinfo/crossfire

Re: [crossfire] Crossfire server code cleanup/janitorial

[Tolga Dalman]

On 04/23/2014 01:19 AM, Kevin Zheng wrote:

I'm still not sure why this would make the project more portable,
though. The major compiler toolchains support C just as well as C++,
with the notable exception of Visual Studio.


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.


I tend to agree, though, I have had some bad experience in another
project with a lot of false-positives using VS 2008. Using GCC, I
would always recommend compiling with -W -Wall.


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.


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.


Right now Crossfire seems maintainable. Recently, several important bug
fixes were made, and few new features were introduced.


As I said before, these developments have inspired me to start this
(quite long) thread. And to repeat: I believe that the overall quality
of the source code is really good. Still there is some way to go.
Thanks for your work!


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.


At the end of the day, if you believe that certain changes are worth
your time making, please send a patch. The recent cleanups were made
because I wanted them and nobody objected to them.


While I really appreciate the positive discussion we have had
up until now, my impression is that I should now be showing you what
I have in mind rather than doing mouth-work only :)
On github, I have created a crossfire server repository to track my
patches (based on SVN trunk rev. 19362).

  https://github.com/tdalman/crossfire

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.


Best regards
Tolga Dalman


___
crossfire mailing list
crossfire@metalforge.org
http://mailman.metalforge.org/mailman/listinfo/crossfire

Re: [crossfire] Crossfire server code cleanup/janitorial

[Nicolas Weeger]

Hello.


   The plugin logic has pluses and minuses - the ability to right plugins
 that register themselves is good, but the entire dynamic loading adds a
 bit of code complexity and another place for errors.  But also, at one
 time, the python plugin was optional, so you could run the server without
 necessarily having python  its development libraries installed.  But
 IIRC, at some point, it was decided that enough scripts and other stuff
 require python it should become standard - since it is known it will
 always exist, whether having it be a dynamically library that is loaded
 makes sense is debatable.

What about simply having compile-time modules?

When you compile, if you have Python then the module is compiled, it not then 
it isn't, end of story :)




Regards


Nicolas


signature.asc
Description: This is a digitally signed message part.
___
crossfire mailing list
crossfire@metalforge.org
http://mailman.metalforge.org/mailman/listinfo/crossfire