Subject: Re: [PATCH] exec: fix inversions in leak detection logic
To: djgpp AT delorie DOT com
References: <964e3268-2f75-ee73-ab5a-b01bf1aadb98 AT yandex DOT ru>
 <qj9iq5$1iqb$2 AT gioia DOT aioe DOT org>
 <19c9edb1-5813-bac2-68dc-59d3fb07d81f AT yandex DOT ru>
 <qja8nc$h45$1 AT gioia DOT aioe DOT org>
From: "stsp (stsp2 AT yandex DOT ru) [via djgpp AT delorie DOT com]" <djgpp AT delorie DOT com>
Message-ID: <4e96d86f-2f80-065a-e760-f1a82534af81@yandex.ru>
Date: Sun, 18 Aug 2019 05:03:16 +0300
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.6.1
MIME-Version: 1.0
In-Reply-To: <qja8nc$h45$1@gioia.aioe.org>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-MW
Content-Transfer-Encoding: 8bit
Reply-To: djgpp AT delorie DOT com
Errors-To: nobody AT delorie DOT com
Precedence: bulk

18.08.2019 4:16, Rod Pemberton пишет:
> On Sat, 17 Aug 2019 22:47:00 +0300
> "stsp (stsp2 AT yandex DOT ru) [via djgpp AT delorie DOT com]" <djgpp AT delorie DOT com>
> wrote:
>
>
>> If the get descriptor call fails
>> we also know this descriptor is currently not user allocated.
>> ---
>> Which means freed!
> No, it doesn't.
>
> Windows - i.e., a non-user - allocates descriptors for itself.

Please, look at the code we discussed:
---
     sel1 = __dpmi_allocate_ldt_descriptors(1);
[...]
       __dpmi_free_ldt_descriptor(sel1);
[...]
         ret = __dpmi_get_descriptor(sel1, &desc_buf2);
         if (ret == -1 || (larbyte != (desc_buf2[5] & 0xf0)))
           workaround_descriptor_leaks = 2;
---
So _they_ allocate the descriptor, then free it, then
see if get descriptor call fails. In which case it means
its freed, because this has nothing to do with the
descriptors allocated by windows... They test descriptors
only allocated by themselves.

>>> I'm confused by this patch, but I'm also not familiar with what the
>>> code does.  Even so, I suspect you fixed the problem in the wrong
>>> manner.  Could you try only modifying the mask to see if that fixes
>>> your issue?
>> This will make thing very DPMI-host-dependent.
>> One host only clears P, another one can do other
>> things.
> DJGPP DPMI apps work for both DOS with DPMI and for Windows in console
> windows with Windows' built-in DPMI for V86 mode.
>
> DJGPP code can already run under many DPMI hosts: CWSDPMI, CWSDPR0,
> PMODETSR, and Windows 98/SE/ME/NT/2K/XP DPMI in console windows, etc.
This comment makes no sense.
We were talking about the possibility to
reduce the mask (exclude P bit), which
is not the right thing to do and will only
shorten the list of supported DPMI servers.

>>> I'm confused by this patch, but I'm also not familiar with what the
>>> code does.
>> In fact, the leaks I am aware of, are coming from
>> the fact that many DPMI servers do not do the
>> automatic clean-up of all resources that were
>> allocated_and not freed_  by the_non-first_  DPMI
>> client.
> I don't know what you mean by this.
>
> AIUI, only one DPMI host is active, the very first one loaded.  AIUI,
> the first DPMI host controls protected-mode (PM) entirely.  Other
> DPMI hosts defer to the first DPMI host,

Great.
Who were talking about multiple dpmi hosts, if the last
word in my quote above is "DPMI client"? I was obviously
and explicitly talking about multiple clients, not hosts. I
don't think you are trying to be helpful.


>   i.e., by not loading or not
> installing.  You seem to be implying that many different DPMI hosts are
> present and functioning,

If you can show anything in my mail that makes you
think I am implying that...


>> I.e. if some client frees his resources - no leak.
> My understanding is that it's only Windows DPMI hosts which leak
> descriptors.

Some early dosemu's-0.x had that too.
In fact, the dpmi in Windows-NT+ have all the bugs that
early dosemu DPMI code have (up to occasionally missing "break"),
so I think they "looked" there a little.
I haven't tested the borland's DPMI with that.


>> Which means, [...]
> Anyway, this issue probably best reviewed by Eli or Charles.
As I said in the mail you replied to, I'll send another patch.
It is now sent, so no more need to review this one.