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

Reply via email to