Brendan wrote:
Hi,
Brendan wrote:
Now all we need is for someone to add comments, so that it's easy to see how bad this code actually is.
It isn't ideal, but it's a hell of a lot better than your ludicrous objections suggest. I will comment on every single negative comment you've made. It'll be fun to add up the score in terms of the good ones versus the bad ones:-
Quote:
;Get byte from keyboard with race conditions (instead of using the BIOS function that is designed to do this right)
Designed only for use with interrupts disabled.
Quote:
If BIOS IRQ handler hasn't already got the byte...
Not possible with interrupts disabled.
Quote:
ready for a broken/idiotic delay...Broken/idiotic delay (may take 0 cycles on some CPUs)...ready for a broken/idiotic delay (contents of CL is undefined)
Delays are not idiotic on the rare type of machine these delays are designed to protect, and CX=0 after the loops run out, so the loop instruction decs it to 15535 before comparing with zero on the kind of machine this is intended to protect. You may not think it possible to damage a machine through aggressive polling, but it most certainly can be done.
Quote:
Broken/idiotic delay (may take 0 cycles on some CPUs)
ditto
Quote:
00007C54 CB retf ;Pointless "retf" for no reason (should be "ret")
Ignorant fool doesn't recognise that it's easier to call a routine with a far call as you don't have to work out the jump distance. This makes it much easier to develop the OS further.
Quote:
;Retarded self-modifying code to screw up CPU's trace cache instead of using a simple variable
How much speed do you think matters at this primitive level while calling the BIOS? What I've gained here is more compact code. I'd have thought you'd have understood the need for that here.
Quote:
Major WTF (there's no sane excuse for IRQs to be disabled by default)
The first thing I did when I wrote my main OS was switch to protected mode, and it was a long time before I bothered with interrupts other than when returning to load/save data. I never bothered to read up on how to get keyboard input from the BIOS as you can't do that from protected mode and I wanted to work in 32-bit mode as much as possible.
Quote:
*facepalm* (this is just a silly wrapper around a retarded "swiss army knife" mess)
That is a means to load and save data efficiently through the BIOS in CHS mode. When I load and save fragmented files in my normal OS I create a bitmap of sectors to be loaded/saved and then generate groups of 6 data bytes of the kind used here to load/save adjacent sectors via the BIOS, thereby making it dead easy to transfer 64KB of data at a time without switching back to protected mode [edit: the point being to make it easier to do the required processing in real mode which I am always keen to keep to a minimum]. It also works in LBA mode, though there are faster ways of doing that as you can load 127 sectors at a time (better to do 64 though) and I use a totally different method to save back to flash drives to reduce wear to the minimum.
Quote:
Continuation of major WTF (there's no sane excuse)
Who wants to write another sodding real-mode OS! Get into protected mode. I ran my OS for years in protected mode with the interrupts disabled because it was all I needed as a platform for running other stuff on that I was developing (which was always more important than the OS).
Quote:
This is part of a jump that jumps to a jump that jumps to a jump(!)
Makes it easier to modify stuff in the boot sector if you're a direct machine code programmer. You aren't, so you wouldn't understand that, but BwtSecOS is self evidently going to require direct machine programming to begin with.
Quote:
should be "jmp near 0x7cfe", or just shift the code at 0x7CFE here to avoid the need for pointless JMPs
My indexing system is designed to work with 32-bit code only so it doesn't automatically adjust the few sections of real-mode code that I use - it would be overkill to write an indexing system for a mode I almost never use, so using one extra (harmless) jump in a bootsector is no great deal. Yes I could save one byte of code, and make my work a lot harder.
Quote:
;Retarded puke to read or write sectors (using BIOS function 0x02, 0x03, 0x42 or 0x43 depending on what was written to 0x7CC2 by self modifying code, even though BIOS functions 0x02 and 0x03 are extremely different to BIOS functions 0x42 and 0x43 and
;none of the logic is shared). Desperately needs to be replaced by a function pointer (e.g. "call [disk_function]").
You made a fuss about me using one extra byte a moment ago and a billionth of a second of extra boot time, but now you want me to use two extra bytes and waste a billionth of a second here instead! The BIOS functions for CHS and LBA are very different, but fully compatible in terms of what you load the registers with before calling the BIOS. I've saved a lot of space by writing the code the way I have.
Quote:
WARNING: This code also uses "lodsb" in a pointless and retarded way instead of doing "mov reg,[address]". This complicates everything for no sane reason.
Have you ever compared the number of machine code bytes used by those instructions? My code is designed for compactness rather than pointless bloat.
Quote:
xchg ax,si ;si = 0x7C29, ax = undefined (should be "mov si,[0x7c0e]" or "mov si,0x7C29")
xchg ax.si uses a single instruction byte. This was done for a very good reason, because when you're learning to program in machine code it takes a while to get up to speed with some of the harder-to-form machine code instructions: anyone trying to use BwtSecOS is likely to prefer doing some indirect loads to begin with, and here I made the machine code more readable for them without losing a byte.
Quote:
WARNING: There's no CLD or STD anywhere, so undefined behaviour here..
That's a good 'un - I've lost a point. Need to add a byte to fix that, and find something that can be compressed, which shouldn't be hard. [Edit: two - need it again after BIOS call.]
Quote:
yes, return with a pointless "RETF" that should be a plain "RET"
Again this is to make it easier to call the procedure without having to work out a sodding jump distance - it's worth losing two bytes for that.
Quote:
ecx = 0 (WARNING: assuming 80386 or later)
Another point to you - I should have warned people.
Quote:
Is some sort of flag zero?
Fetches head value from data. [Edit: no point lost or gained - that's just answering a question.]
Quote:
ax = 0 (should be deleted)
It's function is exactly the same as xor ah,ah here - there's no performance cost, and there may even be a gain.
Quote:
al = something (should be deleted)
The sec_start_no is somewhat vital.
Quote:
add cx,ax ;cx = something (should be "movzx cx,byte [si+0x3]")
My way improves machine code readability.
Quote:
mov al,[si+0x2] ;ax = something (should be "movzx ax,byte [si+0x2]")
Ditto.
Quote:
mov dx,ax ;dx = something (never used, should be deleted)
mov dl,0x24 ;dx = 0x0024 (should be "mov dx,0x0024")
Same end result, but you've saved infinitesimal processor time so take a point for that.
Quote:
(WARNING: highest 16-bits of multiplication in DX are lost)
There aren't >65536 sectors on a floppy disk. [No loss of point though - could be relevant if you aren't restricting yourself to floppy-disk size.]
Quote:
mov eax,[0x7c20] ;ax = 0x00000000 (value never modified)
Maybe that's because it's where someone will manually write in the LBA locaction if BwtSecOS is run from a partition, as explained in the instructions.
Quote:
mov ah,al ;ah = something (should probably be "shl ax,8")
mov al,0x0 ;ax = something << 8 (should probably be "shl ax,8")
Those instructions will cost you an extra byte each time.
Quote:
xchg ax,bx ;bx = something, ax = undefined (should probably be "mov bx,ax")
Machine code programmers' trick to save a byte.
Quote:
xchg ax,dx ;dx = something, ax = undefined (should probably be "mov dx,ax")
Ditto.
Quote:
xchg ax,cx ;ch = cylinder, cl = sector, ax = undefined (should probably be "mov cx,ax" or maybe "mov ch,[cylinder]; mov cl,[sector]")
Ditto.
Quote:
00007CC1 B402 mov ah,0x2 ;WARNING: Value 0x02 is modified by self modifying code
Harmless and saves two bytes.
Quote:
If error, jump to error handler that doesn't do anything to handle the error at all
The error handling code there is incomplete: I never got round to finishing because it did the job well enough for my needs - the key thing it does is check to see if any sectors were loaded, and I've just remembered as I write this that it does that because the BIOS was taking two goes to get to track 0 from track 79, so if it tried to read track 0 when it wasn't there, this was the most economical way of making it have another go at moving there.
Okay then. Who wants to add up the score...?
[Edit: no, it won't be that easy - he'll want to argue this till the cows come home.]