The latest SVN bugs thread

[hendu]

maybe it s little bit slower but that doesn matter here because it s now safer (no goto).

How is it safer?

[zerochen]

never ever in any circumstances use goto. that is not good programming.
eg others can make mistakes with that so quick.
so if you really need a goto because there is no other way to do it then the design is crap and you have to redesign it instead of. there are many papers about goto in the internet. (eg http://www.cs.utexas.edu/~EWD/transcrip ... WD215.html)

here is a failure example:

Code: Select all

                        
goto skip; // invalid forward jump                      
int x = 5;
skip:
x += 3; // what would this even evaluate to?
 

Offtopic:
there is a funny picture about this there a programmer uses goto and a dinosaur runs over him (cant find it right now).

[greenya]

The example you provided is just made by stupid programmer. You may say that nowdays a lot stuff made by teams not a single programmer, so some one just may add "goto" and commit changes so the code you provided may become a life example. That is truth, but the programmer which adds a "goto" must review all the possible execution flow paths -- that is the price and the skill needed to use "goto".

My opinion is to use tools which you know where they are suit best. For example on my work i have a team about 10+ ppl and we are making project which uses a lot of technologies and parts written on different programming languages. We have a code which made by man who just learned about templates in C++, and i can say you that its so huge mess what was done using this powerful tool, the code literally unreadable only because some one very long time ago decided to use them everywhere his hands got to. I don't say that templates are bad (or "goto") -- its a tools, which must be used wisely. If you can write something simplier, you should do it; but if instead of "goto" you redesign the code and the algorithm -- its wrong.

About hendu code, i don't mind at all how it written, i think its OK. He made addition to open source Irrlicht code, and its not some kind of base interfaces which will serve as bricks for future classes, its just a method which does some useful stuff. I would like hendu to fix it him self as he think it should be. I have only wish to the code - it should be compile-able using vs2010 as Irrlicht supports this compiler.

[zerochen]

The example you provided is just made by stupid programmer. You may say that nowdays a lot stuff made by teams not a single programmer, so some one just may add "goto" and commit changes so the code you provided may become a life example. That is truth, but the programmer which adds a "goto" must review all the possible execution flow paths -- that is the price and the skill needed to use "goto".

this was only a little example of cause. but if the function increases every programmer can miss this situation. i wanted to say that you dont have to pay this "price" because there are other solutions. another thing is, that goto is destroying the OOP pattern because you can jump in anywhere you want.

...If you can write something simplier, you should do it; but if instead of "goto" you redesign the code and the algorithm -- its wrong.

i dont redesign it. i only removed the goto cases and add code to handle it because of little time. but i think if someone would redesign it, it become more readable and improvable. also why should that be wrong?

About hendu code, i don't mind at all how it written, i think its OK. He made addition to open source Irrlicht code, and its not some kind of base interfaces which will serve as bricks for future classes, its just a method which does some useful stuff. I would like hendu to fix it him self as he think it should be.

i didnt want to say that wasnt usefulll or something like this. great work indeed but i saw some improvements so i changed it because irrlicht is opensource. and every programmer can commit to irrlicht from beginner to advanced, so it might be risky.

I have only wish to the code - it should be compile-able using vs2010 as Irrlicht supports this compiler.

yes of cause thats why i added the new und delete[] stuff.

in my whole lifetime as a programmer i didnt need one goto statement and i think that isnt bad at all.
i guess we have a different opinion about goto. for you it is just a tool (where somebody must know how to use it) and for me it is just the evil:D

anyway, i didnt want to begin a discussion about goto. i want to offer a fix and a other way to do it.

[hendu]

never ever in any circumstances use goto. that is not good programming.
...
it become more readable and improvable. also why should that be wrong?

We disagree here. Goto can be quite useful in returning errors and freeing resources in proper order without a ton of code duplication, or when you need to exit several levels of loops.

Using code example for the latter:

Code: Select all

for (i = 0;...) {
    for (j = 0; ...) {
        something; goto out;
    }
}
out:

 
This is more readable than

Code: Select all

bool canContinue = true; for (i = 0; i && canContinue...) {
    for (j = 0; ...) {
        something; canContinue = false; break;
    }
    // what if here is some other code that depends on the inner loop?
    // then you need to add an _additional_ if (canContinue) here, making this even more complex
}
 

@greenya

Please test the patch in the linked bug, does it compile in VS?

[greenya]

@hendu
From here http://sourceforge.net/p/irrlicht/bugs/416/
I have tested both patches: CMeshManipulator.cpp.patch and vs-mesh-manip.patch and VS2010 compiles OK with any of them.
I didn't tested the method itself, but only compilation procedure.

[zerochen]

never ever in any circumstances use goto. that is not good programming.
...
it become more readable and improvable. also why should that be wrong?

We disagree here. Goto can be quite useful in returning errors and freeing resources in proper order without a ton of code duplication, or when you need to exit several levels of loops.

Using code example for the latter:

Code: Select all

for (i = 0;...) {
    for (j = 0; ...) {
        something; goto out;
    }
}
out:

 
This is more readable than

Code: Select all

bool canContinue = true; for (i = 0; i && canContinue...) {
    for (j = 0; ...) {
        something; canContinue = false; break;
    }
    // what if here is some other code that depends on the inner loop?
    // then you need to add an _additional_ if (canContinue) here, making this even more complex
}
 

here in your examples you wrote "something;" but often "something;" is more than one line. especially in the opti func it is 3 or more pages. so you have to scroll down to look what out: do and then scroll up. so i think the second solution is more cleaner/readable.

anyway, if somebody has a test case we can check it

[hendu]

@greenya

Thanks. Hybrid, please commit vs-mesh-manip.patch, we still have the other issues under discussion, but that is enough to build under VS.

@zerochen

In the bug you say:

maybe simpler but using of calloc is deprecated / unsafe because it is never throwing exceptions

Irrlicht does not use exceptions. Under these conditions, new is just as unsafe.

Nothing in the C standard is deprecated except gets. MS can think what it wants, sprintf and friends are not deprecated either; in fact, it would be good if MS made their C standard library comply with at least _one_ C standard fully

[hybrid]

Patch applied, brace indentation fixed as well. calloc might be replaced by memset in favor of having a smaller interface to rely on (the memory functions are always problematic). I also disliked the gotos, but didn't see any way to avoid them without making everything much more bloated. So I left them there. The jpeg (and maybe png) readers also have gotos.

[hendu]

Calloc is faster than malloc + memset, though. Which problems do you mean?

[hybrid]

If speed is really an argument, you have to keep the memory allocations out of sight anyway. And calloc seems to be broken or simply missing in many embedded libc implementations. So I usually avoid it completely.

[greenya]

Patch applied...

I have bad news. It is still not compile-able.
The typo is that malloc() indeed takes only one arg and calloc() takes two.

Code: Select all

u32 **accel = (u32 **) malloc(verts, sizeof(u32 *));
// Fix
u32 **accel = (u32 **) malloc(verts * sizeof(u32 *));

[wing64]

CParticleSystemSceneNode.cpp dont deserialize "Affector"
Function: deserializeAttributes Line: 670
FIX

Code: Select all

        if (!name || strcmp("Affector", name))
        {
            ++idx;
            continue;
        }
 

[hybrid]

Patch applied...

I have bad news. It is still not compile-able.
The typo is that malloc() indeed takes only one arg and calloc() takes two.

Code: Select all

u32 **accel = (u32 **) malloc(verts, sizeof(u32 *));
// Fix
u32 **accel = (u32 **) malloc(verts * sizeof(u32 *));

Sorry for that, I guess I should only patch when a compiler is nearby...

[hendu]

Original patch used calloc there