Hi! 30-Июл-2006 22:32 [EMAIL PROTECTED] (Michael Devore) wrote to freedos-devel@lists.sourceforge.net:
>>0. In emm386.c you forget remove one space (in "/ *"). MD> And? It's a comment. Is there another one of those #ifdef 0 type MD> things? Because I'm not ready to debate the style of embedded comments MD> again. Don't debate, just fix. :) BTW, do you know that #if-excluded text can't be _any_ text, but all valid tokens? Ie., if some excluded line contains odd amount (1, 3, etc) of quotes, then this source will/should be complained as wrong. (This is because definition of processing steps, as defined by standards). >>1. Bug: wrong handling for clc/stc in some places (they ignored, because >> popf follows, thus caller receives "always success") ... MD> I found two. Yes. And I quote in original letter fragment of source with labels. MD> out of memory prematurely when free areas were available. Okay, that was MD> worth the entire thread. :) And this is FIXED in 2.21. Though, I don't understand, why to duplicate all pops on each exit branch, whereas you may just move push after other pushes? MD> I'm going to have to nominate you for next MD> month's FreeDOS Contributor of the Month. What flavor gum you want? Nothing. :) Me will be enough bugless programs. :) >>2. Probably, bug: CF not initialized, although comments say so ("probably" >> because I not found in RBIL spec for INT 67/87) ... MD> No such spec. It's a FreeDOS-specific redirect of INT 15, function MD> 87h. Anyway, TEST AH,AH clears the carry flag to known state, the STC sets MD> to known state if error. Everything looks fine to me there. First, where you see "test ah,ah"? Second, see: ______________O\_/_________________________________\_/O______________ cmp cx, word ptr [edi+18h] ja @@invalid_command [...] @@invalid_command: mov ah,80h ret _____________________________________________________________________ O/~\ /~\O Do you know, that JA equal to JNC (_not_ carry set)? Also, more deep analyzing shows, that "@@abort:/mov AH,1/RET" is garbage (not used). BTW, if you check my path, which I send you, you will find there next fragment: ______________O\_/_________________________________\_/O______________ -; jecxz @@ok ; we are done - or cx,cx - je @@ok ; done + clc ; return with CF=0 at CX==0 + jcxz @@ok ; we are done _____________________________________________________________________ O/~\ /~\O See: instead undocumented rely on "OR" behavior, there used shorter code with explicit comments what happen. The more so, if code is redundant, better to remain it as comments. For example, I do this (in CuteMouse) so: ______________O\_/_________________________________\_/O______________ ;stc ; preserved from prev call _____________________________________________________________________ O/~\ /~\O >>3. Bug: missing popf after @@NO_386 ... MD> Fixed, Yes. MD> although I'm not sure a 286 would get there. Does UPX decompressor MD> run under non-386? Yes: "upx --8086". >>4. Misfeature: UMBfree does not merges free blocks. MD> Never intended to. I remember the original explanation for this when it MD> came up (it's not my quote following), and I pretty much agree with it. MD> Paraphrasing heavily: UMB blocks are usually handled by DOS (i.e. people do MD> DOS=UMB). Even when they are not, very few applications would allocate and MD> then deallocate UMBs dynamically, they almost always are allocated at MD> startup and stay attached to the allocating application (for good MD> reason). Upgrading the UMB code for merges etc. was therefore considered MD> an unimportant detail at the time. Why not place this explanation as comment directly into source? >>5. Probably, bug: CF always =0 after XOR, thus, first CheckBlockIntegrity in >> next code never called ("probably" because I not very understand idea of >> given code - thus, first call to CheckBlockIntegrity may be only >> superfluous code, which may be safely removed) ... MD> I removed CheckBlockIntegrity code. You comment it out. But you neither fix neither commented code, nor calling places. Let me remind, that "xor" clears CF, whereas DEC doesn't not, so "call" in "dec/jnc/call CheckBlockIntegrity", which follows "xor al,ah", is unreachable (read: garbage). >>6. Superfluous code (not need to check, if block at [edi] precedes block at >> [esi], because they will be found at next iterations) ... MD> Not superfluous, per previous explanation. It's a very cheap and quick MD> short-circuit, "superfluous" code would be totally without any use. You With the same success you may was unroll those loops. This is also "quick" and "cheap" optimization. MD> mean it could have been done better. Yes. If you replace quoted fragment by one jmp (after merging code to start of internal loop), you get even cheaper solution for even more "short" circuit. ------------------------------------------------------------------------- Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys -- and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV _______________________________________________ Freedos-devel mailing list Freedos-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/freedos-devel