Re: [freesci-develop] Making FreeSCI code C++ friendly

[Max Horn]

Hi folks,

has there been any thoughts on whether to integrate Jordi's patches?  
Of course I am only a bystander, but to me those patches look mostly  
fine (religious C vs. C++ issues aside :-).

In detail:

Am 27.03.2007 um 00:56 schrieb Jordi Vilalta:
[...]
> I've posted the full set of patches here: http://jvprat.no-ip.org/ 
> tmp/freesci/
>
> Here's an overview of the patches:
>
>     01_allocation_casts.patch
> This patch is about the first obvious case where I think the casts
> can't be avoided, and it's
> where allocation of dynamic memory takes place.

This adds some casts to malloc/sci_malloc calls. Maybe not beautiful,  
esp. from the C coder's point of view, who doesn't need them, but the  
patch is totally harmless (and does help a lot to reduce warnings  
when using a C++ compiler on the code).


>     02_kernel_dereference.patch
> This is a bunch of typecasts around kernel_dereferences. I think all
> these should be handled as a block, either by using the explicit
> casting (as it's done in the patch) or by modifying the
> return type of the function (I haven't watched this possibility, but
> as I see most of them cast to (char*)...).

Similiar to patch #1. Could some core developer say whether changing  
the return type would be appropriate?

>     03_flexarray.patch
> Here comes a small patch to fix the FLEX_ARRAY

Tiny patch, also related to type casting. Another harmless but  
helpful one.

>     04_explicit_casts.patch
> This is another relatively big patch. It includes casts where there
> may be a better solution (like changing types of variables), but it
> needs further analysis.

Another cast patch... Yeah, I can see that it might bug of C purists  
to add typecasts they don't need. Ah well :-)

>     05_missing_headers.patch
> This adds a few #includes and a few function declarations where they
> are needed. The function declarations might be moved to include files,
> but in some cases there wasn't an obvious file where to put them, so I
> leave them where they are needed and you can tell me where you want
> them ;)

This one seems like a no-brainer to me (forgive my obvious ignorance  
if I am wrong :-), even for C purists. Function prototypes help catch  
coding mistakes, after all.


>     06_various.patch
> It's a mix of very small changes.

Corrects some small mistakes (in my eyes, at least, returning a value  
in a void function is a mistake :-), see nothing against this patch,  
even for C purists it should be OK, I'd hope.

>     07_c++_keywords.patch
> It renames the C++ keywords that got in the way.

I think this is probably the most controversial part of the patch.  
And I can't understand that. I am not too fond of adding an  
underscore to qualifiers this way. However, let me say two things in  
favor of a *variant* of this patch:

1) it should be easy enough to come up with slightly different names  
which don't look as ugly, e.g.
   new -> create / make / alloc / new_TYPE / ...
   class -> classID / class_id / class_ref / ...
   delete -> dispose / free / release / ...
   etc.

2) While I understand reservations to accept a patch that won't  
benefit the existing developers as such, let's also consider the flip  
side of the coin: It doesn't actually cause damage to existing devs  
either, does it? Other than from a "esthetically" point of view...  
But it *does* make it possible for certain other interested parties  
to work on the FreeSCI engine. And to me as an outsider, it seems  
that you desperately need new blood if you want to get anywhere, as  
(no offense intended) the project appears to be otherwise mostly  
dead / dormant these days and has been so for quite some time  
(years?). At least it appears to be so, and the commit rate (and the  
content of the few commits) seems to support that theory. With  
Jordi's effort, it might soon be possible to make ports of FreeSCI  
for many additional architectures, and maybe this will in turn help  
to attract new blood and invigor the project.

It's your call to decide what coonsider more important, of course! I  
am just an outsider who has nothing to say in these matter (although  
with some experience in related matters on other projects, of  
course :). But let me mention that  *I* don't have to think long to  
decide what I would prefer given the choice... :-)


>     08_cfsml.patch
> The old savegame stuff (for reference).

No comment.

>     09_extra.patch
> Final touch to get everything compilable on C++ (it breaks the C  
> compilation).

Shouldn't go to the repository, of course!

> It's enough for today. I hope this split helps with the integration.

I was hoping so, too, but so far it didn't seem to be the case?



Another thing: A major potentialy portability hurdle for the FreeSCI  
code (regardless of whether a C or C++ compiler is used) is the fact  
that you use system wide #includes w/o path qualifiers. For example,  
on Mac OS X, the system provides vm.h and gc.h headers, so when  
compiling the FreeSCI source w/o explicitly overriding the system  
paths, an error is produced because the wrong headers are included by  
FreeSCI. I wonder if at least the
 #include <...>
statements could be changed to
 #include "..."


Cheers,
Max