This is the mail archive of the cygwin-patches@cygwin.com mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Fix dup for /dev/dsp


At 08:38 PM 7/20/2004 -0400, Christopher Faylor wrote:
>On Tue, Jul 20, 2004 at 11:58:23PM +0200, Gerd Spalink wrote:
>>Yes, I have had a look at archetypes, and I just checked again.
>>What I see is one instance counter (usecount) and a pointer (archetype)
>>and a dependency into cygheap (cygheap->fdtab.find_archetype and others).
>>My patch needs two additional pointers, so in terms of member variables,
>>it is cheap.
>>Also, I don't understand why archetypes work for any close sequence of
>>duped instances?
>
>Given that there have been no complaints about memory corruption, or whatever
>you're anticipating here, I think it is safe to assume that it is working
>ok.
>
>A dup is basically an increment of a use count and a copy of a pointer.
>A close is a decrement of a use count.
>
>>Is the archetype pointer always valid?
>
>As long as the usecount is > 0, yes.
>
>>It it pointing to the heap? and where/when is it freed?
>
>It's pointing into the same place most fhandler objects live -- the
>cygheap.
>
>It's freed (i.e., you free it) when the usecount reaches 0.
>
>>Is the fhandler object's data in the object or on the heap?
>
>See above.
>
>>How to keep consistency after a state change?
>
>Too generic a question.  Can't answer.
>
>>I also found discouraging comments in the code, like
>>  // FIXME: Do this better someday
>
>There are discouraging statements throughout cygwin.  That doesn't mean
>that we stop using signals because sigproc.cc has a couple of FIXMEs.
>
>>Personally, I prefer not to spread dependencies too much, so I would 
>>suggest to leave fhandler_dsp as I suggested in the patch. Most of the
>>changes were done in the ioctl function, which is not affected whether
>>we use archetypes or a linked list.
>
>I don't entirely understand what you are talking about wrt dependencies.
>fhandler_dsp.cc does not exist in an island.  It is part of the rest
>of the cygwin source code base.
>
>I'd rather not have islands of code within cygwin where one person
>decided they could ignore much of the rest of the code.  There have been
>a couple of sections in cygwin like this where a developer didn't really
>understand how the rest of cygwin operated and ended up writing their
>own methods for doing things.  That is something that should be avoided,
>especially since developers drop out from time to time and leave the
>rest of us to scratch our heads over their ideas of how things should be
>done.
>
>cgf

Gert,

The way I view it, the archetype would be used slightly differently
than for ttys. Essentially the 6 private variables of a fhandler_dev_dsp
(those in dup_cpy ) would always be NULL in the fhandler itself,
they would only live in the archetype.

So when an open is successful, just cmalloc an archetype (as for ttys)
and set these variables in the archetype. All references to those variables 
in the fhandler should be replaced by references to the members in the 
archetype. This could be done e.g. as follows for ioctl
fhandler_dev_dsp::ioctl (unsigned int cmd, void *ptr)
 {
   if (archetype)  /* or perhaps if (archetype != this) */
     return ((fhandler_dev_dsp *) archetype)->ioctl (cmd, ptr);
   
   existing code;
}
So a dup boils down to setting the archetype (as for ttys) and
incrementing the count. 
A close would be done as ioctl above, also decrementing the count.
The archetype is cfreed as needed in dtable::release, which is
called from close(). 

Here is another idea.
As noted in your comments, the children cannot change any of the
parameters (because they don't have access to the parent). 
To fix that I am wondering if it wouldn't be better to use a
FileMapping that can be shared between parent and children, 
instead of an archetype.
Apparently that's what fhandler_tape does. See mtinfo_init () in 
fhandler_tape.cc
Perhaps that share can be created on demand when the dsp is opened, 
instead of creating it for every process as tape does.
My understanding is very superficial, I apologize in advance if I 
mislead you.

I have a question: does the testsuite work on 9x/ME? I tried it
for the first time and had a lot of trouble with devdsp.
Eventually I had to power the system down.

Pierre


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]