Irrlicht and C++11

[kormoran]

I managed to successfully compile Irrlicht with TDM mingw64 5.2.0 and -std=c++11.
All I needed to do was some small patches to the aesGladman code.

In aesGladman/sha2.h:
from this

Code: Select all

#define n_u32(p)    sfx_hi(0x##p,s_u32)
#define n_u64(p)    sfx_hi(0x##p,s_u64)
/* define an unsigned 32-bit type */
// etc. etc... define sha2_32t
/* define an unsigned 64-bit type */
// etc. etc... define sha2_64t
 

to this:

Code: Select all

#define n_u32(p)    0x##p
#define n_u64(p)    0x##p
 
typedef irr::u32   sha2_32t;
typedef irr::u64   sha2_64t;

(could also easily get rid of both macros, but anyway...)

in aesGladman/aeskey.cpp:
anything like

Code: Select all

cx->ks[4] = ff(ss[4] = word_in(in_key, 4));
 

to

Code: Select all

ss[4] = word_in(in_key, 4);
cx->ks[4] = ff(ss[4]);

Hope this can be useful to anyone out there

[CuteAlien]

Thanks. I'll take a look. Did you just remove the sfx_hi part? Well, I'll have to check I guess what this is about :-)

I also got on my plan to fix the aes warnings and maybe update the lib (if I can find a newer version). But might be a while until I get to that (but is one of the open tasks before releasing Irrlicht 1.9).

[kormoran]

Thanks for the info

Seems to me that the "sfx_hi()" and "sfx_lo()" macros are not used anywhere, so removing them should be safe.
However I did the bare minimum to get along... one can easily get rid of n_u32()/n_u64() too, if really want to.

I already noted that many warnings are placeholders for things to come, so I worked only on what is pointlessly annoying.
More patches are on the way now that I know where to put them (already fixed almost all aes warnings haha )

[hendu]

Did you test aes still worked?

[kormoran]

Did you test aes still worked?

Uhm, no... but as I tried to test it this morning, came out that aes code is not (yet) used in Irrlicht or at least, no source includes anything from aesGladman.

Maybe aes is there because someone plans to use it someday. However all I did was to eliminate some type guessing, since irrlicht already does that task (more reliably than twelve years old code can), so there shouldn't be any difference...

[CuteAlien]

Aes is used in zip-loading for password protected zip's. I think there is a test in the tests folder (but tests don't run through in trunk right now because of texture-locking changes not yet done, so can only be tested by modifiying the main file in tests manually).

Also have to check if there is a newer AES version. It's a little tricky in this case, as unlike with the other libs we modified AES a lot. And I might have applied some patches once I got from Gladman after reporting a problem as well and I'll have to compare if those made it in the official version already (if I even find that one... I had a little trouble finding those which we need when I looked recently).

[kormoran]

Ach so.

I browsed some irrlicht code this morning. Zlib, pnglib, jpeglib, aes, every added library tries to guess its own version of irr::u8, irr::u16, irr::s16, irr::u32, boolean etc... some even trying to guess if they are on MSDOS and/or need FAR pointers

This could become a problem, since old assumptions tends to be false with new compilers, as we have just seen with aes. I see why you plan to get a newer version of aes, but probably even that will redo the same guessing work irrlicht does on its own.

On the other hand, they are so stable (well working, tested and no new features needed) that I think could be a good idea to assimilate them in Irrlicht. With "assimilate" I mean let them all use irrTypes.h for their typedefs, ditch all the guessing and state squarely they aren't on MSDOS, VMS or whatever God-forgotten system, let them use irr::logger instead of stderr and so on. This would make irrlicht codebase less prone to troubles when supporting new compilers/OSes, and lessen the maintaining work in the long run. What do you think about it?

[CuteAlien]

The problem with doing that is that it's makes updating vendor libs a lot harder. Coding that stuff once is always easy - but the burden of maintenance would increase a lot. I think the reason it was done for AES was likely that it was hard to get away without integrating it. But generally it's a bad idea. It already makes AES very hard to update right now compared to the other libs. So we try to minimize changes as much as we can.

Also it should still be possible to switch out most of those libs with system libs. That's especially important for Linux distributions.

I recently started documenting the changes which had been made (in svn vendor branch - vendorlibs.txt) and I'm really glad there hadn't been made more changes. Because figuring out those already took me a day (and AES still mostly open because all the changes there make it now tricky).

The correct solution is usually to get the original authors to change their libraries if it's something which really has to be changed.
Or alternatively - implementing it from scratch, but that also increases the amount of code to maintain.

[CuteAlien]

Just noticed we also have to fix those errors for gcc6: http://people.canonical.com/~doko/tmp/gcc6-regr/ (search for Irrlicht, then gunzip the file twice(!) to get the real file).
gcc6 also has a nice new -Wmisleading-indentation warning :-)

[kormoran]

Hm, just great... Probably they enable that warning when setting warnlevel to "dictatorship" or some like it
However I just compiled Irrlicht (patched) with std=c++14: success, it's all OK.

Pondering on what you said before, I reviewed my patch.
The same fix can be achieved modifying a single line in sha2.h (line 79) from

Code: Select all

    typedef int64_t sha2_64t;

to

Code: Select all

    typedef uint64_t sha2_64t;

That was surely an error, since comment right above the line states clearly that sha2_64t is to be declared as unsigned
(edited, thanks AReichl)

[AReichl]

it's in sha2.h (line 79)

[CuteAlien]

Yeah, I'll check it once I work on updating that lib. If it's an error then I it should probably be fixed in the original lib (if it's there as well). Looks a lot like that's it.

[CuteAlien]

I've added some changes in trunk which hopefully fix it. For the type I just used uint64_t instead of int64_t (seemed the smallest possible change). For some reason it had compiled here even with -std=c++11 but I suppose when all warnings are gone then c++11 should also work.