Dumbest mistake you have ever made

[Solar]

My answer always is something to the effect of, "I cannot tell you the deep magic of the code, you must search for the meaning yourself because in the journey lies the answer." Then I let out an evil laugh and run away.

That's why I have a 9mm semi-automatic in my drawer - to shoot people like you in the back when they run like that, to leave their corpses rotting in the hallway, forgotten by the world and bemoaned by no-one.

The magic mantra here, of course, is "comment your code, comment your code, comment your code..."

PS: No smiley above... while I of course don't slay co-workers as a habit, sometimes I wish somebody would, as I don't really feel like smiling in moments like this.

[babernat]

That's why I have a 9mm semi-automatic in my drawer - to shoot people like you in the back when they run like that, to leave their corpses rotting in the hallway, forgotten by the world and bemoaned by no-one.

The magic mantra here, of course, is "comment your code, comment your code, comment your code..."

Really? What brand? Mine is a Sig P228. Although mine sits at home.

I note that sarcasm is difficult to to detect in forum posts and rely on the fact that you've never programmed with me before. I put more effort than has been given to me in regards to helping other programmers. It seems much of my time is spent cleaning up flaming piles of turd left in the wake of hacks.

[inflater]

That's why I have a 9mm semi-automatic in my drawer

Let me guess... Beretta? H&K? Glock 18? Luger P08?

I bet it's Glock.

[Candy]

The magic mantra here, of course, is "comment your code, comment your code, comment your code..."

We had a discussion on bad code recently at work. One of the first items that propped up was "using comments" and our conclusion was "if you use comments for your code, it isn't clear enough so you should rewrite it - so using comments is essentially a bad thing". We made an exclusion for compatibility comments such as "we used this odd method to be compatible with XYZ" but no more than that.

[Solar]

Some code comment examples from PDCLib, to prove my point instead of ranting about it:

Code: Select all

/* Note that, while n is unsigned and can thus be larger than INT_MAX, the
   return value cannot, so this function will never write more than INT_MAX
   characters (which it is allowed to do).
*/
int _PDCLIB_write( struct _PDCLIB_file_t * stream, char const * buffer, _PDCLIB_size_t n )

Code: Select all

    /* This implicit typecast ssize_t -> int is made implicit *on purpose*, so
       that a good compiler can give a warning if ssize_t should *not* resolve
       to int - in which case this function's code must be reconsidered anyway
       because we cannot use the ssize_t of the POSIX function write() for a
       PDCLib return code.
    */

Code: Select all

        /* Now we did the padding, do the prefixes (if any). */
        preidx = 0;
        while ( preface[ preidx ] != '\0' )
        {
            DELIVER( preface[ preidx++ ] );
            ++(status->this);
        }
        if ( ( ! ( status->flags & E_minus ) ) && ( status->flags & E_zero ) )
        {
            /* If field is not left aligned, and zero padding is requested, do
               so.
            */
            while ( status->this < status->width )
            {
                DELIVER( '0' );
                ++(status->this);
            }
        }
        /* Do the precision padding if necessary. */
        for ( size_t i = 0; i < prec_pads; ++i )
        {
            DELIVER( '0' );
        }

And I prefer the H&K P7.

[JackScott]

There seem to me to be two types of comments: How comments, and why comments.

How comments, as Candy said, indicate that your code need rewriting.
IMHO, why comments tell others (and yourself) vital info on why you did something. They should be in there.

[Candy]

Code: Select all

/* Note that, while n is unsigned and can thus be larger than INT_MAX, the
   return value cannot, so this function will never write more than INT_MAX
   characters (which it is allowed to do).
*/
int _PDCLIB_write( struct _PDCLIB_file_t * stream, char const * buffer, _PDCLIB_size_t n )

Code: Select all

int write(...) {
    precondition(n <= INT_MAX);
    ...
}

Code: Select all

    /* This implicit typecast ssize_t -> int is made implicit *on purpose*, so
       that a good compiler can give a warning if ssize_t should *not* resolve
       to int - in which case this function's code must be reconsidered anyway
       because we cannot use the ssize_t of the POSIX function write() for a
       PDCLib return code.
    */

Valid comment - compatibility with standard

Code: Select all

        /* Now we did the padding, do the prefixes (if any). */
        preidx = 0;
        while ( preface[ preidx ] != '\0' )
        {
            DELIVER( preface[ preidx++ ] );
            ++(status->this);
        }
        if ( ( ! ( status->flags & E_minus ) ) && ( status->flags & E_zero ) )
        {
            /* If field is not left aligned, and zero padding is requested, do
               so.
            */
            while ( status->this < status->width )
            {
                DELIVER( '0' );
                ++(status->this);
            }
        }
        /* Do the precision padding if necessary. */
        for ( size_t i = 0; i < prec_pads; ++i )
        {
            DELIVER( '0' );
        }

Code: Select all

        char *prefix = preface;
        while ( prefix )
        {
            DELIVER( *prefix );
            ++prefix;
        }
#define BIT_SET(status, flag) (((status)->flags) & (flag)) == flag)
        if ( ! BIT_SET(status, E_minus) && BIT_SET(status, E_zero))
        {
            while ( status->current < status->significand_width )
            {
                DELIVER( '0' );
                ++(status->current);
            }
        }

        for ( size_t i = 0; i < precision_padding_length; ++i )
        {
            DELIVER( '0' );
        }

Keeping with most of your logic.

I prefer well-written code to any kind of weaponry, except in all-out war or target practice.

[Solar]

Code: Select all

int write(...) {
    precondition(n <= INT_MAX);
    ...
}

Not equivalent. It is perfectly OK to give a n > INT_MAX as parameter, but the function will not write more than INT_MAX, because it couldn't correctly fit that into the return value.

Actually, POSIX write() does exactly the same - just undocumented, which annoyed me a great deal when I had to wrap that function.

Keeping with most of your logic.

Some good improvements in there, but I would still keep the comments. My usual modus operandi is that I am looking for the part of code that does something special, like the zero padding. The code does tell me "how", and you can usually deduct the "what" from that (and your variable naming does a better job than mine making that step easier). But I still prefer a comment telling me the "what" (and perhaps the "why") right away, because it makes it quicker finding out this isn't the code section you're looking for after all.

But we had that argument before.

[mystran]

Probably one of the dumpest mistakes I've done is actually a loop involving realtime audio processing with denormal numbers.. that is, multiplying each sample repeatedly with something like 0.9 until the processor runs out of precision... the problem being that ... well you can try, if you don't mind your computer freezing to the point where it doesn't bother moving your mouse cursor anymore.. technically not totally frozen ofcourse.. things still happen eventually... but I got tired of waiting for the task manager to come up after mm.. something like 30 minutes..

The other dumpest thing is a little replication script I wrote just a while back, to copy changed files over ssh to another server. Had to do it without installing software on the target system.. hence a self made script. Anyway, that's not the dump part.. nor is it dump that the script worked perfectly (and still does). The dump part is that I wrote a cron script to check once every minute that the replication script would be alive..

The cronscript looked something like:

if ps aux | grep -v scriptname; then nohup scriptname &; fi

Now, ofcourse teh problem was that the part of the scriptname that I grepped for, wouldn't fit in the 80 characters that "ps aux" was giving me by default. Two days later the server was getting a bit slow.. "there's half a million bash and ssh running, could you do something with it" said me phone..

Changed it to 'ps auxwww' and it works like it's supposed to.

[mystran]

Not equivalent. It is perfectly OK to give a n > INT_MAX as parameter, but the function will not write more than INT_MAX, because it couldn't correctly fit that into the return value.

There's no way to make a bug out of that anyway, as read() and write() are specified to return what they wrote, which can be any number less than or equal to the length of the buffer given. You could put the limit at 42 and it'd still work as specified. In general it's stupid to expect that they'd write the whole buffer.

[Solar]

That code was taken out of fflush() (or rather, a subfunction thereof)...

[Candy]

That code was taken out of fflush() (or rather, a subfunction thereof)...

My idea behind transforming it to that precondition was - if you give it a buffer of more than that, you're likely doing something wrong. The library will not fill the expectation you would otherwise have (as shown by Solar's comment which would not give the user an indication) but it will work - which essentially means as far as I'm concerned that it doesn't fulfill the interface.

Worst case, size_t is usually half the adressable space of the system. Only on 16-bit systems with segmentation and huge 32-bit systems that allow for buffers of more than half of that would the argument make any sense. As far as I'm aware, 16-bit systems are very rare (and not your target) and 32-bit systems never (afaik) support more than 2GB in a single buffer since the malloc would've failed too.

[Steve the Pirate]

Probably the dumbest programming mistake I ever made was learning Visual Basic...

[AndrewAPrice]

I rewrote my page manager, and as soon as my kernel enters it identity maps the lower/top 1GB using 4MB pages in assembly. Once in the C++ kernel, my proper page tables are created using 4KB pages.

After a week of trying to discover why Bochs kept triple faulting, I finally discovered that I kept the 4MB pages bit set in my page directory entries. I found about a dozen other bugs along the way though

EDIT: Still on the paging topic, I was loading ELF files into memory and I was getting "Invalid Opcode" (VirtualBox sucks for catching this exception BTW, it crashes rather than handle it through my OS's exception system). I spent most of this afternoon looking through my thread constructor and my scheduler wondering why. Then I decided to print out the memory it's trying to execute which happened to be "255 255 255 255 255 255" (a.k.a. 0xFF 0xFF etc in decimal), even though I just loaded perfectly valid instructions into that location! Eventually I tried:

Code: Select all

char *a = (char *)1024 * 1024;
*a = 100;
console << (unsigned int) a << " "; 

but it was still printing out "255"! That's when I knew something was fishy with my paging, and it just happened that when I was allocating the pages to load the program into, I created a page table, and set the page table's page directory entry to the virtual address of the page table, not the physical!
It was the difference between:

Code: Select all

processPageTable[pagetable][pagetableentry] = (unsigned int)(pageAddrPhysical * MEMORY_PAGESIZE) | 3;

and

Code: Select all

processPageTable[pagetable][pagetableentry] = (unsigned int)(pageAddr * MEMORY_PAGESIZE) | 3;

That waisted me an afternoon of debugging while I ripped apart half of my kernel.

[maverick777]

Total newb mistake comming up here , basically after having asm code in my kernel from a guide, well there was a few routines that displayed the operating systems welcome message :

msg db 0x0A, 0x0A, "- OS Development Series -"
db 0x0A, 0x0A, " MOS 32 Bit Kernel Executing", 0x0A, 0

Anyway the routine putch32 (as example) takes ebx as a param which has the address of the start of the msg variable. I decided I want to see if I understand the code so write a simpler function(Not so well written but to aid understanding) to display the first letter of
tst db 0x0A,"way hey",0

I adjusted all the code wandering why I was getting a weird result for the first char for hours and it turns out offcourse the first char is a new line lol not the w letter I was expecting

Very newbie type error I know so Im guessing Ive much more fun to come hehe (Shudders to think!)

Another dumb mistake unrelated to OS dev was when I played swg wrote a macro and it really messed up, went into an infinite loop and started flooding server chat lol