Back to home page

DOS ain't dead

Forum index page

Log in | Register

Search:  
Back to the forum
Board view  Mix view

review (Developers)

posted by Ninho E-mail, 23.05.2011, 23:10

So many, detailed, comments, wow !

Just to put things into perspective, since I started modifying Japheth's program incrementally, I'm like a painter retouching an existing picture rather than one drawing onto a fresh canvas. I just sit in front of my frame, uh, screen and type code in the text editor, without separate notes of any kind. Now, the mods have taken us much farther from the starting point than I first envisioned. And since it is also my pleasure to deliver things even in an unfinished (yet I hope somewhat usable) state, I am aware of imperfections like unused, left over, variables (I've removed some but not all).

>> assemble with MASM (I used 6.15)

> Would it work with JWASM too? I don't really care right now, though if I
> ever wanted to assemble it...
No idea, welcome to try. I went out of my way myself to download MASM, since this is what Japheth wrote for, usually I program in TASM.

>> important is to present the objects in the following order
> Shouldn't it be possible to make it independent of the order if you
> relocate the entire process? Depending on the memory layout it might be
> possible.
I find it more elegant to relocate the resident in one move, and not have to ask for an auxiliary memory block, yielding complications if DOS couldn't allocate one (since the resident part is so small it may be not be a problem in practice but it might when installing high into a small umb for instance).

>> optional bit of hokus-pokus to adjust and shrink the header of the
> resulting EXE
> Just use flat .COM executables if the .EXE header size worries you ;-)
There are several ways to skin that cat. Yep, my pref would have gone to a .com too. Especially using MASM as opposed to TASM, I'm not always comfortable with the assembler's choice of segment: (or group:) origin for offset calculation especially when we start moving segments around... As a consequence I feel obliged to doublecheck the binaries in the debugger; I must say this MASM 6.15 has suprised me favorably as being not too "quirky",
I remembered older versions of the microsoft assembler doing strange unpredictable things sometimes :=)

> Now here's what I determined during testing:
> The "drvflgs" variable in the Int2F resident part is apparently not used.
True : legacy.

> I do not see why the "psp" variable has to be in the resident part, either.
> It's apparently used with 21.49 by the deinstall function, however, isn't
> it sufficient if the function determines itself which memory block to
> free?
The badly named "psp", really the resident segment value, is returned by the communication routine (int 2F). Legacy again.

Altogether that whole int 2F hook is doing little useful and it even renders uninstallation more problematic than need be; it is going to disappear, clearing the above 2 remarks.
I'll have another method for uninstalling, plus an option for disabling without removal.

> I see that you do relocate your PSP and all. Why didn't you choose to
> change the UMB link status and allocation strategy while allocating the
> memory block for the resident part then? I consider a program's ability to
> relocate itself into a UMB a feature.
This version was delivered to test and evaluate the "load low" option. It had all UMB management removed.

> It's true that DOS doesn't use the PSP beyond offset 50h. I don't see a
> reason not to relocate the entire PSP though.
Elegance for one. And memory usage, as I would have to enlarge the stack correspondingly (hence the whole executable memory footprint) if I were to copy the entire PSP. 50h bytes may be even too much for our short lived PSP here ;=) I guess 40h bytes would suffice (we don't call DOSVER :)

Ah, now comes the more serious stuff :

> I prefer activating the new PSP via process termination. Theoretically, if
> any resident program associates resources with our PSP address, my method
> might prevent leaks. In practice it probably doesn't matter though.
That's splitting hair. Would a foreign TSR hijack our PSP ? Any interrupting TSR which allocates resources for itself must activate its *own* PSP first, it would seem. It's not like we are *calling* a TSR as a service

> You use 21.4A to resize the part of the process which contains the old PSP.
> This means your new PSP is in free memory after DOS's MCB resizing
> finished.
Yep, well spotted!

> (It doesn't matter that even the DOS algorithm could overwrite
> your new PSP since any resident code interrupting your program's run at
> that time is allowed to allocate and overwrite the memory block anyway.)
I'm convinced. I could do direct MCB fixes, like you suggest under b) later, and like, I think, I did in earlier attempts years back. But this time I found it interesting to try and put DOS functions to work whenever possible rather than direct manipulation of structures.

This bug I think is simply the order of the steps. Simple fix (referring to the numbered comments in the code) : do steps 3.2 & 3.3 /before/ step 3. In fact I can't believe I couldn't see this before submitting.

> The best solutions to this are in my opinion: (a) do not split memory
> blocks at all, instead, relocate the entire process to an entirely new
> memory block,

avoided for reasons already told

> (b) if you choose to split memory blocks, do not use this
> unsafe 21.4A method; instead, prepare the new MCB yourself and modify your
> old MCB to point to that one....
Easily done, yet I believe 21/4A will be safe if done in the revised order.

> You free your process's memory block partially after relocating the PSP.
> However, you have not relocated your program code. Your installer
> executes in free memory after that. This is far worse than the previous
> issue because you do issue 21.48 (to allocate the TSR's memory block)
> yourself while executing code in free memory. If DOS happened to allocate a
> memory block overlapping your program the latter might get overwritten.
> Note that the critical part of your program which must not be overwritten
> here includes the TSR's image that you copy into its block later.
This is serious. My own use of 48h is not a danger, size of the requested block is accounted for so it won't trash anything still needed.
OTOH you're right asynchronous allocation (by a TSR ?) could, in theory, ruin the game after we free most of our own container (at #3.4)

I was aware of this but after (too little) thought, it didn't seem to matter... and since in practice nothing exploded I counted on the review process, honest !

Step 3.4 (freeing our old, previously shrinked, container) appeared necessary because allocation of the resident block (step 3.5) must be allowed to step over the start of our program. That was the central idea. I wanted to avoid having to request extra memory, as already said.

Thinking aloud, I could, instead of freeing all of my container, free the first (size of resident) bytes only, by direct MCB handling, that would reduce the surface of the problem you have uncovered but not solve it in theory. Is there a way to make : (free, then allocate) into a "critical section" of sorts ? I don't suppose "CLI" would suffice, would it ?

> The 21.48 request to allocate memory for your resident program definitively
> needs an error check.
I don't think so, since the steps I've taken ensure this allocation can't fail. An error check can't hurt anyway (you saw the comments) ;=)

> Why do you set the TSR's MCB to "SC", owner 8? I prefer to set my TSR's
> MCBs so I can distinguish them: self-owned with a more or less descriptive
> name.
This was left as per Japheth. If it turns out later on that we have to walk the MCBs for some reason, we may revise it. As we don't ATM it's a no brainer.

> Related insignificant nitpick: Your new PSP's MCB's name is uninitialized.
> That's of course not important since that name is never visible except in
> debuggers.
and except when debugging, the PSP in question lives only for a fraction of a second anyway :=)

> Even if you fixed the issues with using free memory, your method might fail
> to place the TSR at the optimal position if your resident memory
> requirements grew larger

Can't happen, not a limitation for this application.

> Other insignificant nitpicks: Why do you calculate the size of the TSR at
> run time? I see that you have to dispatch between the value for the
> removable TSR and the non-removable one, but that still doesn't explain why
> you don't use the assembler to do the shifting and such at build time.
Code reuse again !

> Similarly, why did you hardcode the "32 bytes" message?
It's only printed by the test Kbzero version, I think.

> assembler calculate the memory block size difference between the removable
> and non-removable TSR and write that into the message.
Moot point since that particular option is going away and the resident size will be fixed, probably.


Did I thank you for the review, comments and time passed ? Much appreciated!

---
Ninho

Post reply
 

Complete thread:

Back to the forum
Board view  Mix view
22049 Postings in 2034 Threads, 396 registered users, 234 users online (0 registered, 234 guests)
DOS ain't dead | Admin contact
RSS Feed
powered by my little forum