-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Vince L wrote: > On Saturday 02 June 2007 04:22, azeem ahmad wrote: >> hi list >> i am about to make a bootable floppy for test >> but i am being unable to get it done >> please review the code below and tell me if there is any problem with it > > Very funny!!! I have recently left a job in part because I became very tired > of reviewing rubbish code in assembler when I felt certain I could write the > same thing in C and leave less problems if my C was unreviewed than there > would be if I reviewed someone else's misguided assembler. >
Any high level computer language comes with overheads on the compiled code, and I have seen in the past seen some truly horrible low level code generated by high level language compilers. Also on occasion in the past I have found decompiling to assembler and hand optimisation was a necessity. Some assembler code is truly ugly (using 100s of NOPS for timing is one example I can think off), where timing or speed is essential one will nearly always get a better result with well written assembler (it often is not pretty to look at at).. One would assume in your role as a code reviewer you were not only taking into account what the code did but how well it did it. (If not why not?). <snip> >> call _disp # call subroutine disp > Useless comment. Says what you can see from the code > >> _disp: #label of the disp routine > That too is a useless comment. Again, either we know it's a label, or we have > nothing useful to contribute to a review. No comment is needed here, it just > makes the rest harder to read. > >> lodsb # load the string pointed by ds:si in a >> # - byte by byte manner into the accumulator > This is a misleading comment. You should be shot for this. AFAICS, this > instruction loads just 1 byte, it does not do anything byte by byte - it is > the _disp loop as a whole which does byte by byte. > >> jz ret # jump if al zero to return > Another useless comment for the same reason. I am not hot on Intel assembler, Obviously not, see below! jz checks the status of zero flag > so I don't know it is al which is zero compared - but I do know that to > review this I would have to know what jz does - so as reviewer, I know it is > my responsibility either to understand each line or to find out what it does > >> or %al, %al # check if the entire string has been loaded byte-wise >> # by oring al to al, >> # if the result is 0, there are no more bytes to load Really useful ... swapped the order of instructions .. or al,al will set the Z flag to zero for the jz test. As you have ordered them this routine could go on for ever. If you do not know about it, dont comment on it. > This is a far more useful comment, because it reveals strategy - ie what you <snip> This a bog standard Intel interrupt 10h display routine which could be lifted out of any 8086 assembler cookbook. It does and should not need any more comment that. Comments are a matter a style, personally I dislike end of line comments (they get in the way of the code), and prefer block head comments. However, for a novice, which the guy obviously is, the comments are probably a good mnemonic. My suspicion is the original got mauled in transmission. The only comment I made on it was a suspicion that a dangerous assumption was being made with the string definition, which is probably an easily made mistake by a novice assembler programmer with a C background. > > If your intention is to make a bootable floppy, you may find this link > useful: > http://linuxgazette.net/issue84/dashti.html. Good luck with your coding - but > even if you are fixing the comments in this code, you've had your 5 minutes > from me. May be this is not the help you expected - I have been been hard on > your style - but I hope that it will help you in the future. > I would suspect that he attempting first steps in using assembler to communicate with the PC BIOS. Something which is usually rather difficult to do on both the Linux and Win32 platform. Using 8/16 bit intel assembler in useful start point, rather than the 32 bit instruction set. There are also quite a view 8088/8086 devices out there in the process control world. -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.5 (GNU/Linux) Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org iD8DBQFGYpH2asN0sSnLmgIRAs2ZAKCWlvIqzBZYRx2g8ouf4aZKBPaVOQCgy4tN 9feZSHVXJuVkYkkk4FWLqEM= =RJnh -----END PGP SIGNATURE----- -- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
