Dumbest coding mistakes ever


void *operator new(std::size_t size, stackalloc_t stackalloc) {
void *p = _malloca(size);
if (!p)
throw std::bad_alloc();
return p;
}


void *operator new(std::size_t size, stackalloc_t stackalloc) {
void *p = _malloca(size);
if (!p)
throw std::bad_alloc();
return p;
}

Is this your code? I can see that you'r e not using stackalloc, what side effect does this have? malloca still allocs on the stack right?

using /W4 usually saves me from stuff like this, although ive never had the need to override new

When writing OpenGL shaders. And I did this twice in a row. Go me:
void (main) {
...
}

Also not mine, but something that was mentioned on a forum a couple of years ago (ironically said forum just shut down an hour ago or so Not kidding). Some old J2ME phones had a limit on how long functions could be, so sometimes functions had to be split. Somebody once was told to divide such a function in two. Well, guess what he did...
public void Paint() / 2
{
...
}

I think you missed the point -

When writing OpenGL shaders. And I did this twice in a row. Go me:
void (main) {
...
}

Also not mine, but something that was mentioned on a forum a couple of years ago (ironically said forum just shut down an hour ago or so Not kidding). Some old J2ME phones had a limit on how long functions could be, so sometimes functions had to be split. Somebody once was told to divide such a function in two. Well, guess what he did...
public void Paint() / 2
{
...
}

I think you missed the point - both my mistakes and mikep's mistake compiled just fine but caused problems at runtime.

I cant see the code, but this happened to me a few hours ago:


How fucked up is that? im guessing some kind of stack corruption or something (the nuke should take 60 seconds)

[quote name='Mike.Popoloski' timestamp='1328578594' post='4910373']

void *operator new(std::size_t size, stackalloc_t stackalloc) {
void *p = _malloca(size);
if (!p)
throw std::bad_alloc();
return p;
}

Is this your code? I can see that you'r e not using stackalloc, what side effect does this have? malloca still allocs on the stack right?

using /W4 usually saves me from stuff like this, although ive never had the need to override new
[/quote]

_malloca does indeed allocate on the stack. Which means the memory allocated is effectively "freed" the moment this operator returns, and will be reused by whatever the parent function calls, or even the parent function itself.

I don't remember exactly where I did this, but I did manage to pull this one off once:
int& derp = derp;

And here's a more recent moment of weakness:
if ( a != b ) a < b;

[color=#000000]And I've tried to track down performance problems multiple times in the past year with my debugger attached... with conditional breakpoints inside inner loops...

languages with C like syntax allow you full control of layout etc, but this can cause bugs, like this one that kept me busy for 2 days. I had been refactoring my resource loading system and removed something from a render list class into something else, then gone for a cup of tea, leaving behind this code:


public void AddItem(Matrix position, IRenderable3D i)
{
// if we have run out of space, reallocate with 2x the amount of space
if (CurrentIndex >= capacity - 1)
Grow(2);
if(! i.GetMaterial().Linked)
this.
renderables[CurrentIndex].absolute_position = position;
renderables[CurrentIndex].item = i;
CurrentIndex++;
}


Spot the error.

This is the classic argument for significant whitespace ala Python/Haskell. Why leave something as fundamental as the structure of your code to convention? Sure, brackets parse unambigiously enough, but then your visual cortex still has to pick up on them; it usually just relies on the indentation.

This is the classic argument for significant whitespace ala Python/Haskell. Why leave something as fundamental as the structure of your code to convention? Sure, brackets parse unambigiously enough, but then your visual cortex still has to pick up on them; it usually just relies on the indentation.

Fair point, but for most of the stuff I've worked on coding standards had pretty strict rules on visually formatting the code. Stuff like this wouldn't pass code review for me:
[source]
if (CurrentIndex >= capacity - 1)
Grow(2);
if(! i.GetMaterial().Linked)
this.

[/source]

It would be sweet if you could make VS throw warnings for code that doesn't fit standards. You might be able to do that already, but I don't know how to do that :/

[quote name='Eelco' timestamp='1328602267' post='4910440']
This is the classic argument for significant whitespace ala Python/Haskell. Why leave something as fundamental as the structure of your code to convention? Sure, brackets parse unambigiously enough, but then your visual cortex still has to pick up on them; it usually just relies on the indentation.

Fair point, but for most of the stuff I've worked on coding standards had pretty strict rules on visually formatting the code. Stuff like this wouldn't pass code review for me:
[source]
if (CurrentIndex >= capacity - 1)
Grow(2);
if(! i.GetMaterial().Linked)
this.

[/source]

It would be sweet if you could make VS throw warnings for code that doesn't fit standards. You might be able to do that already, but I don't know how to do that :/
[/quote]

This is why I usually have a policy of not using control structures without a block, but sometimes I relapse. It would be good to have a plugin or something that could detect this.

This is why I usually have a policy of not using control structures without a block, but sometimes I relapse. It would be good to have a plugin or something that could detect this.

I think the standard we're working with now allows single line if statements like if(a
Personally, I always use blocks, so that's why I'm not positive about the above in our coding standard. I only ever don't use them with the ternary operator, and I only use the ternary operator when I'm conditionally initializing stuff because it's harder to visually parse and some people have no idea what it is.

I don't remember exactly where I did this, but I did manage to pull this one off once:
int& derp = derp;

It annoys me that this is even allowed in the language.

These are my "dumbest" moments, in C++ at least:

struct Foo
{
Foo(float u, float v) : u(u), v(u)
{
}

float u;
float v;
};


A related version:


struct Bar
{
Bar(float someVariableX, float someVairableY)
:
someVariableX(someVariableX),
someVariableY(someVariableY)
{
}

float someVariableX;
float someVariableY;
};



lotsAndLotsAndLotsOfCode();
for(int i = 0 ; i < N ; +i) {
lotsAndLotsAndLotsOfCode();
frobnicate(i);
lotsAndLotsAndLotsOfCode();
}
lotsAndLotsAndLotsOfCode();

The common problem was dismissing the problematic code as being too trivial to contain a bug, and spending lots of time debugging the calling/called code looking for errors. One gets too used to reading what one expects to be the case, rather than reading what is actually there.

I do this every once in a while. Luckily I've gotten pretty quick at catching it in the debugger, but that's only because I've gotten practice:

for (int i = someStartValue; i > 0; ++i)
{
// code
}

I know I make plenty of others, but I'll have to try and remember them...

I guess I am just a bizarre abberation. I write all my code with every line on the left edge

thing=0;
otherthing=10;
while(thing<otherthing) {
stuff;
stuff;
thing++;
}
more;
code;
all;
on;
the;
left;


spacing things out generally confuses the hell out of me too.

I guess I am just a bizarre abberation. I write all my code with every line on the left edge

thing=0;
otherthing=10;
while(thing<otherthing) {
stuff;
stuff;
thing++;
}
more;
code;
all;
on;
the;
left;


spacing things out generally confuses the hell out of me too.

Yup, that's pretty weird. But there's no mistakes in teh codes....

I was writing a Windows service and needed to change the privileges of the process token.

SetPrivileges("SE_RESTORE_NAME\0SE_BACKUP_NAME\0SE_TCB_NAME\0\0");

The function would split up the string and LookupPrivilegeValue() for each privilege, building up the structure to pass to AdjustTokenPrivileges().
This worked in the previous version of Windows I was using it (I don't remember if it was XP or Vista), but in Windows 7 it didn't; LookupPrivilegeValue() was failing.

Couldn't figure it out until I looked at WinNT.h:
#define SE_RESTORE_NAME TEXT("SeRestorePrivilege")

The fix, obviously, is

SetPrivileges(SE_RESTORE_NAME "\0" SE_BACKUP_NAME "\0" SE_TCB_NAME "\0\0");

What I still don't fully understand is why it worked in older Windows 7.


while( someCondition );
{
}


I often flip the direction of boolean tests. Like it should be

if (foo) ...

and I'll write

if (!foo) ...

These can be hard things to catch, because you can stare at the bad comparison but your brain just kind of assumes that the call knows what it is doing.

[quote name='speciesUnknown' timestamp='1328628754' post='4910526']
This is why I usually have a policy of not using control structures without a block, but sometimes I relapse. It would be good to have a plugin or something that could detect this.

I think the standard we're working with now allows single line if statements like if(a
Personally, I always use blocks, so that's why I'm not positive about the above in our coding standard. I only ever don't use them with the ternary operator, and I only use the ternary operator when I'm conditionally initializing stuff because it's harder to visually parse and some people have no idea what it is.
[/quote]

Having your own standards is a partial solution; but it breaks down when you have to use some random persons code, and you are back to wishing the language enforced a standard.

Trying to abuse special constructors instead of delegating the work to functions .


class Map2D
{
Map2D() { makeGenericMap(); }
Map2D( int width, int height, int cell_size, int layers )
{
makeMap(width, height, cell_size, layers);
}
...
}
...
class SpecificMap : public Map2D
{
SpecificMap() : Map2D()
{
MapParser parser("filename");

if( parser->isGood() )
{
Map2D::Map2D(parser->getWidth(), parser->getHeight(), ... ); // Nothing would ever happen

....

// More stuff

}
}
}



I thought it'd be the same as calling any other super-function at the time. Don't know what in the world I was thinking. Spent a good bit of time tracing it down because "I thought it'd just work like that."

I often flip the direction of boolean tests. Like it should be

if (foo) ...

and I'll write

if (!foo) ...

These can be hard things to catch, because you can stare at the bad comparison but your brain just kind of assumes that the call knows what it is doing.

The worst part is when the boolean variable name adds to the logic.

if (notRunning) {
notRunning = isRunning();
}