Re: time for 1.5.11? (/dev/dsp problems -- Gerd take note)

[Christopher Faylor <me at cgf dot cx>]

Gerd,
What do you think about Igor's findings?

cgf

On Wed, Jun 09, 2004 at 08:56:13PM -0400, Igor Pechtchanski wrote:
>On Wed, 9 Jun 2004, Christopher Faylor wrote:
>
>> On Wed, Jun 09, 2004 at 04:28:37PM -0400, Igor Pechtchanski wrote:
>> >On Wed, 9 Jun 2004, Igor Pechtchanski wrote:
>> >
>> >> On Wed, 9 Jun 2004, Corinna Vinschen wrote:
>> >>
>> >> > On Jun  9 13:01, Christopher Faylor wrote:
>> >> > > Anyway, on to the future...  Would it make sense to release a 1.5.11?
>> >> > > Do you think we've hit all of the fallout from the path reorg/speedup?
>> >> >
>> >> > Looking through the reports on the Cygwin list, I see that we never
>> >> > got a reply if the MapViewOfFileEx problem has been solved.  Also,
>> >> > what about the report that /dev/dsp isn't working?  Igor was going to
>> >> > investigate this further.  Any news, Igor?
>> >>
>> >> I'm still investigating.  So far it looks like /dev/dsp doesn't like to be
>> >> dup'd (but then, who does? ;-)).  I'm attaching a short test program to
>> >> demostrate this, but I'll keep digging...
>> >
>> >...And moments after I sent the above I found the culprit (I think).
>> >Seems like fhandler_dsp::dup() neglects to call fhandler_base::dup(), so
>> >the IO handle is unset.  It should be a trivial 1-line patch, but just in
>> >case, can someone with a copyright assignment take care of this?
>>
>> You should be safe if it is really a one line patch.  Don't you want to
>> get your name into the Cygwin changelog limelight?
>>
>> cgf
>
>Ok, it turns out that to fix this properly, we'll need a much larger patch
>than the one line I was looking at...  Here are some thoughts.
>
>First off, my guess above was wrong.  The IO handle has nothing to do with
>this -- fhandler_dev_dsp doesn't use it.  However, the intuition (I still
>think) was correct: some fields, namely the audio_in_ and audio_out_
>pointers, weren't replicated on dup(), so the new (duped) object ended up
>with NULL fields, and couldn't write to them.
>
>The simplest solution to the above is to simply copy the pointers.  This
>will work for the test program I provided earlier, but won't for another
>legitimate test program (attached) that closes the original handle before
>using the duped one (I think this behavior is allowed).
>
>The "proper" way to fix this, IMO, would be to allow sharing the audio_in_
>and audio_out_ objects, and have some sort of reference count in them,
>instead of deleting them outright in close().  I'm not sure this will
>cover the situations when a process is spawned (e.g., fork), but it should
>suffice for redirection.  This, however, promises to be a non-trivial
>patch.
>
>Comments?
>	Igor
>-- 
>				http://cs.nyu.edu/~pechtcha/
>      |\      _,,,---,,_		pechtcha@cs.nyu.edu
>ZZZzz /,`.-'`'    -.  ;-;;,_		igor@watson.ibm.com
>     |,4-  ) )-,_. ,\ (  `'-'		Igor Pechtchanski, Ph.D.
>    '---''(_/--'  `-'\_) fL	a.k.a JaguaR-R-R-r-r-r-.-.-.  Meow!
>
>"I have since come to realize that being between your mentor and his route
>to the bathroom is a major career booster."  -- Patrick Naughton
>#include <fcntl.h>
>#include <unistd.h>
>
>#define BUFSIZE	1024
>#define WORKS	"This works\n"
>#define NWORKS	"This doesn\'t work\n"
>
>int main(int ac, char *av[]) {
>  char buf[BUFSIZE+1];
>  int num, src, dst;
>
>  write(2, WORKS, sizeof(WORKS)-1);
>  src = open("/cygdrive/c/WINNT/Media/tada.wav", O_RDONLY);
>  dst = open("/dev/dsp", 0x601);
>  while ((num = read(src, buf, BUFSIZE)) > 0)
>    write(dst, buf, num);
>  close(dst);
>  close(src);
>
>  write(2, NWORKS, sizeof(NWORKS)-1);
>  src = open("/cygdrive/c/WINNT/Media/tada.wav", O_RDONLY);
>  dst = open("/dev/dsp", 0x601);
>  if (dup2(dst, 1) != -1) {
>    close(dst);
>    dst = 1;
>    while ((num = read(src, buf, BUFSIZE)) > 0)
>      write(dst, buf, num);
>  }
>  close(dst);
>  close(src);
>}
>