[ros-diffs] [janderwald] 49238: [PORTCLS] - Fix check if the pin can be instantiated another time - Remove hack to close old - Add function FreePin to remove its old reference - Fix memory leaks / reference le...

janderwald at svn.reactos.org janderwald at svn.reactos.org
Sat Oct 23 12:10:57 UTC 2010


Author: janderwald
Date: Sat Oct 23 12:10:56 2010
New Revision: 49238

URL: http://svn.reactos.org/svn/reactos?rev=49238&view=rev
Log:
[PORTCLS]
- Fix check if the pin can be instantiated another time
- Remove hack to close old
- Add function FreePin to remove its old reference
- Fix memory leaks / reference leaks in WavePci pin implementation
- Fix memory / reference leaks in WavePci Close implementation

Modified:
    trunk/reactos/drivers/wdm/audio/backpln/portcls/filter_wavecyclic.cpp
    trunk/reactos/drivers/wdm/audio/backpln/portcls/filter_wavepci.cpp
    trunk/reactos/drivers/wdm/audio/backpln/portcls/interfaces.hpp
    trunk/reactos/drivers/wdm/audio/backpln/portcls/pin_wavecyclic.cpp
    trunk/reactos/drivers/wdm/audio/backpln/portcls/pin_wavepci.cpp
    trunk/reactos/drivers/wdm/audio/backpln/portcls/port_wavecyclic.cpp

Modified: trunk/reactos/drivers/wdm/audio/backpln/portcls/filter_wavecyclic.cpp
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/wdm/audio/backpln/portcls/filter_wavecyclic.cpp?rev=49238&r1=49237&r2=49238&view=diff
==============================================================================
--- trunk/reactos/drivers/wdm/audio/backpln/portcls/filter_wavecyclic.cpp [iso-8859-1] (original)
+++ trunk/reactos/drivers/wdm/audio/backpln/portcls/filter_wavecyclic.cpp [iso-8859-1] Sat Oct 23 12:10:56 2010
@@ -327,7 +327,7 @@
 NTSTATUS
 NTAPI
 CPortFilterWaveCyclic::FreePin(
-    IN struct IPortPinWaveCyclic* Pin)
+    IN PPORTPINWAVECYCLIC Pin)
 {
     ULONG Index;
 

Modified: trunk/reactos/drivers/wdm/audio/backpln/portcls/filter_wavepci.cpp
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/wdm/audio/backpln/portcls/filter_wavepci.cpp?rev=49238&r1=49237&r2=49238&view=diff
==============================================================================
--- trunk/reactos/drivers/wdm/audio/backpln/portcls/filter_wavepci.cpp [iso-8859-1] (original)
+++ trunk/reactos/drivers/wdm/audio/backpln/portcls/filter_wavepci.cpp [iso-8859-1] Sat Oct 23 12:10:56 2010
@@ -96,12 +96,13 @@
         return STATUS_UNSUCCESSFUL;
     }
 
-    if (m_Pins[ConnectDetails->PinId] && m_Descriptor->Factory.Instances[ConnectDetails->PinId].CurrentPinInstanceCount)
-    {
-        // release existing instance
-        PC_ASSERT(0);
-        m_Pins[ConnectDetails->PinId]->Close(DeviceObject, NULL);
-    }
+    if (m_Pins[ConnectDetails->PinId] && 
+        (m_Descriptor->Factory.Instances[ConnectDetails->PinId].CurrentPinInstanceCount == m_Descriptor->Factory.Instances[ConnectDetails->PinId].MaxFilterInstanceCount))
+    {
+        // no available instance
+        return STATUS_UNSUCCESSFUL;
+    }
+
 
     // now create the pin
     Status = NewPortPinWavePci(&Pin);
@@ -305,6 +306,26 @@
     return STATUS_SUCCESS;
 }
 
+NTSTATUS
+NTAPI
+CPortFilterWavePci::FreePin(
+    IN struct IPortPinWavePci* Pin)
+{
+    ULONG Index;
+
+    for(Index = 0; Index < m_Descriptor->Factory.PinDescriptorCount; Index++)
+    {
+        if (m_Pins[Index] == Pin)
+        {
+            m_Descriptor->Factory.Instances[Index].CurrentPinInstanceCount--;
+            m_Pins[Index] = NULL;
+            return STATUS_SUCCESS;
+        }
+    }
+    return STATUS_UNSUCCESSFUL;
+}
+
+
 NTSTATUS 
 NewPortFilterWavePci(
     OUT IPortFilterWavePci ** OutFilter)

Modified: trunk/reactos/drivers/wdm/audio/backpln/portcls/interfaces.hpp
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/wdm/audio/backpln/portcls/interfaces.hpp?rev=49238&r1=49237&r2=49238&view=diff
==============================================================================
--- trunk/reactos/drivers/wdm/audio/backpln/portcls/interfaces.hpp [iso-8859-1] (original)
+++ trunk/reactos/drivers/wdm/audio/backpln/portcls/interfaces.hpp [iso-8859-1] Sat Oct 23 12:10:56 2010
@@ -599,6 +599,8 @@
 #undef INTERFACE
 #define INTERFACE IPortFilterWavePci
 
+struct IPortPinWavePci;
+
 DECLARE_INTERFACE_(IPortFilterWavePci, IIrpTarget)
 {
     DEFINE_ABSTRACT_UNKNOWN()
@@ -607,6 +609,9 @@
 
     STDMETHOD_(NTSTATUS, Init)(THIS_
         IN PPORTWAVEPCI Port)PURE;
+
+    STDMETHOD_(NTSTATUS, FreePin)(THIS_
+        IN struct IPortPinWavePci* Pin)PURE;
 };
 
 typedef IPortFilterWavePci *PPORTFILTERWAVEPCI;
@@ -614,7 +619,10 @@
 #define IMP_IPortFilterPci           \
     IMP_IIrpTarget;                         \
     STDMETHODIMP_(NTSTATUS) Init(THIS_      \
-        IN PPORTWAVEPCI Port)
+        IN PPORTWAVEPCI Port);              \
+    STDMETHODIMP_(NTSTATUS) FreePin(THIS_   \
+        IN struct IPortPinWavePci* Pin)
+
 
 /*****************************************************************************
  * IPortPinWavePci

Modified: trunk/reactos/drivers/wdm/audio/backpln/portcls/pin_wavecyclic.cpp
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/wdm/audio/backpln/portcls/pin_wavecyclic.cpp?rev=49238&r1=49237&r2=49238&view=diff
==============================================================================
--- trunk/reactos/drivers/wdm/audio/backpln/portcls/pin_wavecyclic.cpp [iso-8859-1] (original)
+++ trunk/reactos/drivers/wdm/audio/backpln/portcls/pin_wavecyclic.cpp [iso-8859-1] Sat Oct 23 12:10:56 2010
@@ -960,27 +960,29 @@
     {
         // free format
         FreeItem(m_Format, TAG_PORTCLASS);
+
+        // format is freed
         m_Format = NULL;
     }
 
     if (m_IrpQueue)
     {
-        // fixme cancel irps
+        // cancel remaining irps
+        m_IrpQueue->CancelBuffers();
+
+        // release irp queue
         m_IrpQueue->Release();
-    }
-
-
-    if (m_Port)
-    {
-        // release reference to port driver
-        m_Port->Release();
-        m_Port = NULL;
+
+        // queue is freed
+        m_IrpQueue = NULL;
     }
 
     if (m_ServiceGroup)
     {
         // remove member from service group
         m_ServiceGroup->RemoveMember(PSERVICESINK(this));
+
+        // do not release service group, it is released by the miniport object
         m_ServiceGroup = NULL;
     }
 
@@ -999,20 +1001,35 @@
         // set state to stop
         m_State = KSSTATE_STOP;
 
-
         DPRINT("Closing stream at Irql %u\n", KeGetCurrentIrql());
+
         // release stream
         m_Stream->Release();
 
+        // stream is now freed
+        m_Stream = NULL;
     }
 
 
     if (m_Filter)
     {
-        // release reference to filter instance
+        // disconnect pin from filter
         m_Filter->FreePin((PPORTPINWAVECYCLIC)this);
+
+        // release filter reference
         m_Filter->Release();
+
+        // pin is done with filter
         m_Filter = NULL;
+    }
+
+    if (m_Port)
+    {
+        // release reference to port driver
+        m_Port->Release();
+
+        // work is done for port
+        m_Port = NULL;
     }
 
     Irp->IoStatus.Information = 0;

Modified: trunk/reactos/drivers/wdm/audio/backpln/portcls/pin_wavepci.cpp
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/wdm/audio/backpln/portcls/pin_wavepci.cpp?rev=49238&r1=49237&r2=49238&view=diff
==============================================================================
--- trunk/reactos/drivers/wdm/audio/backpln/portcls/pin_wavepci.cpp [iso-8859-1] (original)
+++ trunk/reactos/drivers/wdm/audio/backpln/portcls/pin_wavepci.cpp [iso-8859-1] Sat Oct 23 12:10:56 2010
@@ -61,8 +61,6 @@
     ULONG m_TotalPackets;
     KSAUDIO_POSITION m_Position;
     ULONG m_StopCount;
-
-    ULONG m_Delay;
 
     BOOL m_bUsePrefetch;
     ULONG m_PrefetchOffset;
@@ -637,41 +635,85 @@
     IN PDEVICE_OBJECT DeviceObject,
     IN PIRP Irp)
 {
-    ISubdevice *SubDevice;
     NTSTATUS Status;
-    PSUBDEVICE_DESCRIPTOR Descriptor;
+
+    if (m_Format)
+    {
+        // free format
+        FreeItem(m_Format, TAG_PORTCLASS);
+
+        // format is freed
+        m_Format = NULL;
+    }
+
+    if (m_IrpQueue)
+    {
+        // cancel remaining irps
+        m_IrpQueue->CancelBuffers();
+
+        // release irp queue
+        m_IrpQueue->Release();
+
+        // queue is freed
+        m_IrpQueue = NULL;
+    }
+
 
     if (m_ServiceGroup)
     {
+        // remove member from service group
         m_ServiceGroup->RemoveMember(PSERVICESINK(this));
+
+        // do not release service group, it is released by the miniport object
+        m_ServiceGroup = NULL;
     }
 
     if (m_Stream)
     {
         if (m_State != KSSTATE_STOP)
         {
-            m_Stream->SetState(KSSTATE_STOP);
+            // stop stream
+            Status = m_Stream->SetState(KSSTATE_STOP);
+            if (!NT_SUCCESS(Status))
+            {
+                DPRINT("Warning: failed to stop stream with %x\n", Status);
+                PC_ASSERT(0);
+            }
         }
+        // set state to stop
+        m_State = KSSTATE_STOP;
+
+        DPRINT("Closing stream at Irql %u\n", KeGetCurrentIrql());
+
+        // release stream
         m_Stream->Release();
-    }
-
-    Status = m_Port->QueryInterface(IID_ISubdevice, (PVOID*)&SubDevice);
-    if (NT_SUCCESS(Status))
-    {
-        Status = SubDevice->GetDescriptor(&Descriptor);
-        if (NT_SUCCESS(Status))
-        {
-            Descriptor->Factory.Instances[m_ConnectDetails->PinId].CurrentPinInstanceCount--;
-        }
-        SubDevice->Release();
-    }
-
-    if (m_Format)
-    {
-        FreeItem(m_Format, TAG_PORTCLASS);
-        m_Format = NULL;
-    }
-
+
+        // stream is now freed
+        m_Stream = NULL;
+    }
+
+    if (m_Filter)
+    {
+        // disconnect pin from filter
+        m_Filter->FreePin((PPORTPINWAVEPCI)this);
+
+        // release filter reference
+        m_Filter->Release();
+
+        // pin is done with filter
+        m_Filter = NULL;
+    }
+
+    if (m_Port)
+    {
+        // release reference to port driver
+        m_Port->Release();
+
+        // work is done for port
+        m_Port = NULL;
+    }
+
+    // successfully complete irp
     Irp->IoStatus.Status = STATUS_SUCCESS;
     Irp->IoStatus.Information = 0;
     IoCompleteRequest(Irp, IO_NO_INCREMENT);
@@ -756,42 +798,66 @@
     NTSTATUS Status;
     PKSDATAFORMAT DataFormat;
     BOOLEAN Capture;
-
+    ISubdevice * Subdevice = NULL;
+    PSUBDEVICE_DESCRIPTOR SubDeviceDescriptor = NULL;
+
+    // check if it is a source / sink pin
+    if (KsPinDescriptor->Communication == KSPIN_COMMUNICATION_SINK && KsPinDescriptor->DataFlow == KSPIN_DATAFLOW_IN)
+    {
+        // sink pin
+        Capture = FALSE;
+    }
+    else if (KsPinDescriptor->Communication == KSPIN_COMMUNICATION_SINK && KsPinDescriptor->DataFlow == KSPIN_DATAFLOW_OUT)
+    {
+        // source pin
+        Capture = TRUE;
+    }
+    else
+    {
+        DPRINT("Unexpected Communication %u DataFlow %u\n", KsPinDescriptor->Communication, KsPinDescriptor->DataFlow);
+        DbgBreakPoint();
+        while(TRUE);
+    }
+
+    // add port / filter reference
     Port->AddRef();
     Filter->AddRef();
 
+    // initialize pin
     m_Port = Port;
     m_Filter = Filter;
     m_KsPinDescriptor = KsPinDescriptor;
     m_ConnectDetails = ConnectDetails;
     m_Miniport = GetWavePciMiniport(Port);
     m_DeviceObject = DeviceObject;
-
+    m_State = KSSTATE_STOP;
+    m_Capture = Capture;
+
+    DPRINT("IPortPinWavePci_fnInit entered\n");
+
+    // get dataformat
     DataFormat = (PKSDATAFORMAT)(ConnectDetails + 1);
 
-    DPRINT("IPortPinWavePci_fnInit entered\n");
-
+    // allocate data format
     m_Format = (PKSDATAFORMAT)AllocateItem(NonPagedPool, DataFormat->FormatSize, TAG_PORTCLASS);
     if (!m_Format)
+    {
+        // release references
+        m_Port->Release();
+        m_Filter->Release();
+
+        // no dangling pointers
+        Port = NULL;
+        Filter = NULL;
+
+        // failed to allocate data format
         return STATUS_INSUFFICIENT_RESOURCES;
-
+    }
+
+    // copy data format
     RtlMoveMemory(m_Format, DataFormat, DataFormat->FormatSize);
 
-    if (KsPinDescriptor->Communication == KSPIN_COMMUNICATION_SINK && KsPinDescriptor->DataFlow == KSPIN_DATAFLOW_IN)
-    {
-        Capture = FALSE;
-    }
-    else if (KsPinDescriptor->Communication == KSPIN_COMMUNICATION_SINK && KsPinDescriptor->DataFlow == KSPIN_DATAFLOW_OUT)
-    {
-        Capture = TRUE;
-    }
-    else
-    {
-        DPRINT("Unexpected Communication %u DataFlow %u\n", KsPinDescriptor->Communication, KsPinDescriptor->DataFlow);
-        DbgBreakPoint();
-        while(TRUE);
-    }
-
+    // allocate new stream
     Status = m_Miniport->NewStream(&m_Stream,
                                    NULL,
                                    NonPagedPool,
@@ -805,46 +871,80 @@
     DPRINT("IPortPinWavePci_fnInit Status %x\n", Status);
 
     if (!NT_SUCCESS(Status))
+    {
+        // free references
+        Port->Release();
+        Filter->Release();
+
+        // free data format
+        FreeItem(m_Format, TAG_PORTCLASS);
+
+        // no dangling pointers
+        m_Port = NULL;
+        m_Filter = NULL;
+        m_Format = NULL;
+
+        // failed to allocate stream
         return Status;
-
-    if (m_ServiceGroup)
-    {
-        Status = m_ServiceGroup->AddMember(PSERVICESINK(this));
-        if (!NT_SUCCESS(Status))
-        {
-            DPRINT("Failed to add pin to service group\n");
-            return Status;
-        }
-    }
-
-    // delay of 10 milisec
-    m_Delay = Int32x32To64(10, -10000);
-
+    }
+
+    // get allocator requirements for pin
     Status = m_Stream->GetAllocatorFraming(&m_AllocatorFraming);
+    if (NT_SUCCESS(Status))
+    {
+        DPRINT("OptionFlags %x RequirementsFlag %x PoolType %x Frames %lu FrameSize %lu FileAlignment %lu\n",
+               m_AllocatorFraming.OptionsFlags, m_AllocatorFraming.RequirementsFlags, m_AllocatorFraming.PoolType, m_AllocatorFraming.Frames, m_AllocatorFraming.FrameSize, m_AllocatorFraming.FileAlignment);
+    }
+
+    // allocate new irp queue
+    Status = NewIrpQueue(&m_IrpQueue);
     if (!NT_SUCCESS(Status))
     {
-        DPRINT("GetAllocatorFraming failed with %x\n", Status);
-    }
-
-    DPRINT("OptionFlags %x RequirementsFlag %x PoolType %x Frames %lu FrameSize %lu FileAlignment %lu\n",
-           m_AllocatorFraming.OptionsFlags, m_AllocatorFraming.RequirementsFlags, m_AllocatorFraming.PoolType, m_AllocatorFraming.Frames, m_AllocatorFraming.FrameSize, m_AllocatorFraming.FileAlignment);
-
-    ISubdevice * Subdevice = NULL;
+        // free references
+        Port->Release();
+        Filter->Release();
+        m_Stream->Release();
+
+        // free data format
+        FreeItem(m_Format, TAG_PORTCLASS);
+
+        // no dangling pointers
+        m_Port = NULL;
+        m_Filter = NULL;
+        m_Format = NULL;
+        m_Stream = NULL;
+
+        // failed to allocate irp queue
+        return Status;
+    }
+
+    // initialize irp queue
+    Status = m_IrpQueue->Init(ConnectDetails, m_AllocatorFraming.FrameSize, m_AllocatorFraming.FileAlignment, NULL);
+    if (!NT_SUCCESS(Status))
+    {
+        // this should never happen
+        ASSERT(0);
+    }
+
     // get subdevice interface
     Status = Port->QueryInterface(IID_ISubdevice, (PVOID*)&Subdevice);
 
     if (!NT_SUCCESS(Status))
-        return Status;
-
-    PSUBDEVICE_DESCRIPTOR SubDeviceDescriptor = NULL;
-
+    {
+        // this function should never fail
+        ASSERT(0);
+    }
+
+    // get subdevice descriptor
     Status = Subdevice->GetDescriptor(&SubDeviceDescriptor);
     if (!NT_SUCCESS(Status))
     {
-        // failed to get descriptor
-        Subdevice->Release();
-        return Status;
-    }
+        // this function should never fail
+        ASSERT(0);
+    }
+
+    // release subdevice
+    Subdevice->Release();
 
     /* set up subdevice descriptor */
     RtlZeroMemory(&m_Descriptor, sizeof(SUBDEVICE_DESCRIPTOR));
@@ -855,21 +955,30 @@
     m_Descriptor.UnknownMiniport = SubDeviceDescriptor->UnknownMiniport;
     m_Descriptor.PortPin = (PVOID)this;
 
-
-
-    Status = NewIrpQueue(&m_IrpQueue);
-    if (!NT_SUCCESS(Status))
-        return Status;
-
-    Status = m_IrpQueue->Init(ConnectDetails, m_AllocatorFraming.FrameSize, m_AllocatorFraming.FileAlignment, NULL);
-    if (!NT_SUCCESS(Status))
-    {
-        DPRINT("IrpQueue_Init failed with %x\n", Status);
-        return Status;
-    }
-
-    m_State = KSSTATE_STOP;
-    m_Capture = Capture;
+    if (m_ServiceGroup)
+    {
+        Status = m_ServiceGroup->AddMember(PSERVICESINK(this));
+        if (!NT_SUCCESS(Status))
+        {
+            // free references
+            m_Stream->Release();
+            Port->Release();
+            Filter->Release();
+
+            // free data format
+            FreeItem(m_Format, TAG_PORTCLASS);
+
+            // no dangling pointers
+            m_Stream = NULL;
+            m_Port = NULL;
+            m_Filter = NULL;
+            m_Format = NULL;
+
+            // failed to add to service group
+            return Status;
+        }
+    }
+
 
     return STATUS_SUCCESS;
 }

Modified: trunk/reactos/drivers/wdm/audio/backpln/portcls/port_wavecyclic.cpp
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/wdm/audio/backpln/portcls/port_wavecyclic.cpp?rev=49238&r1=49237&r2=49238&view=diff
==============================================================================
--- trunk/reactos/drivers/wdm/audio/backpln/portcls/port_wavecyclic.cpp [iso-8859-1] (original)
+++ trunk/reactos/drivers/wdm/audio/backpln/portcls/port_wavecyclic.cpp [iso-8859-1] Sat Oct 23 12:10:56 2010
@@ -275,12 +275,12 @@
     }
 
     // create the subdevice descriptor
-    Status = PcCreateSubdeviceDescriptor(&m_SubDeviceDescriptor, 
+    Status = PcCreateSubdeviceDescriptor(&m_SubDeviceDescriptor,
                                          4,
                                          InterfaceGuids,
-                                         0, 
+                                         0,
                                          NULL,
-                                         2, 
+                                         2,
                                          WaveCyclicPropertySet,
                                          0,
                                          0,




More information about the Ros-diffs mailing list