Correct way to handle a NULL ppPins parameter in DirectShow Filter's EnumPins method (Delphi/DSPACK)?

Question

I have a custom push source filter written in Delphi 6 using the DSPACK DirectShow component library on a Windows XP/32 machine. During coding I ran into a problem with range check errors occurring in the BaseClass unit belonging to DSPACK (BaseClass.pas), after I turned on Range Checking in my application's Compiler options.

The error is occurring during my the EnumPins() method. Note, this method resides in DSPACK's BaseClass unit, not my application. I traced the problem and discovered it occurs when the Filter graph I build that uses my filter, plays the filter. Note, my filter is being directly incorporated into my application as a private unregistered filter not as an external AX. When DirectShow calls into the base class method TBCEnumPins.Next(), if the ppPins parameter is NIL then the range check error occurs. Since I am not a DirectShow expert, I am not sure what is the correct way to fix this error without disturbing the proper flow of the DirectShow pin enumeration process. If instead it is a true error condition that is not to be ignored, then I need to know what is the correct Exception to throw or HRESULT code to return in this event. Can anyone tell me the proper way to adjust this code for a NIL ppPins parameter? The full method code is below with the line at which the range check error occurs, highlighted:

function TBCEnumPins.Next(cPins: ULONG; out ppPins: IPin; pcFetched: PULONG): HRESULT;
type
  TPointerDynArray = array of Pointer;
  TIPinDynArray = array of IPin;
var
  Fetched: cardinal;
  RealPins: integer;
  Pin: TBCBasePin;
begin
    // ATI: Debugging range check error.
    try
        if pcFetched <> nil then
          pcFetched^ := 0
        else
          if (cPins>1) then
          begin
            result := E_INVALIDARG;
            exit;
          end;
        Fetched := 0; // increment as we get each one.

        // Check we are still in sync with the filter
        // If we are out of sync, we should refresh the enumerator.
        // This will reset the position and update the other members, but
        // will not clear cache of pins we have already returned.
        if AreWeOutOfSync then
          Refresh;

        // Calculate the number of available pins
        RealPins := min(FPinCount - FPosition, cPins);
        if RealPins = 0 then
        begin
          result := S_FALSE;
          exit;
        end;

        {  Return each pin interface NOTE GetPin returns CBasePin * not addrefed
           so we must QI for the IPin (which increments its reference count)
           If while we are retrieving a pin from the filter an error occurs we
           assume that our internal state is stale with respect to the filter
           (for example someone has deleted a pin) so we
           return VFW_E_ENUM_OUT_OF_SYNC }

        while RealPins > 0 do
        begin
          // Get the next pin object from the filter */
          inc(FPosition);
          Pin := FFilter.GetPin(FPosition-1);
          if Pin = nil then
          begin
            // If this happend, and it's not the first time through, then we've got a problem,
            // since we should really go back and release the iPins, which we have previously
            // AddRef'ed.
            ASSERT(Fetched = 0);
            result := VFW_E_ENUM_OUT_OF_SYNC;
            exit;
          end;

          // We only want to return this pin, if it is not in our cache
          if FPinCache.IndexOf(Pin) = -1 then
          begin
            // From the object get an IPin interface
            TPointerDynArray(@ppPins)[Fetched] := nil; // <<<<<< THIS IS WHERE THE RANGE CHECK ERROR OCCURS.
            TIPinDynArray(@ppPins)[Fetched] := Pin;
            inc(Fetched);
            FPinCache.Add(Pin);
            dec(RealPins);
          end;
        end; // while RealPins > 0 do

        if (pcFetched <> nil) then pcFetched^ := Fetched;

        if (cPins = Fetched) then result := NOERROR else result := S_FALSE;
    except
        On E: Exception do
        begin
            OutputDebugString(PChar(
                '(TBCEnumPins.Next) Exception class name(' + E.ClassName + ') message: ' + E.Message
            ));

            raise;
        end;
    end;
end;

UPDATE: It appears that the DSPACK code is sound from a technical standpoint, but a little odd from a coding idiom viewpoint and is structured in a way that is not compatible with range checking. The NIL coming in via the ppPins "out" parameter maps to the destination buffer the caller passes to TBCEnumPins.Next() as the ppPins parameter. For example, the code below comes from this page:

http://tanvon.wordpress.com/2008/09/07/enumerating-the-directshow-filter-pin/

On that page is the following code that interacts with a DirectShow Filter to enumerate the Filter's pins:

IEnumPins * pEP;
pSF->EnumPins(&pEP);
IPin * pOutPin;
while(pEP->Next(1,&pOutPin,0) == S_OK)
{
    PIN_DIRECTION pDir;
    pOutPin->QueryDirection(&pDir);
    if(pDir == PINDIR_OUTPUT)
        break;// success
    pOutPin->Release();
}
pEP->Release();

By telling the Next() method how many pins to retrieve, the TBCEnumPins.Next() method code with its unusual Dynamic array cast works out to be safe since it will only copy into the ppPins "out" parameter as many as pins as were requested in the Next() functions "cPins" parameter. As long as the caller passes a destination buffer that can hold the number of pins requested in "cPins" everything works fine (as long as range checking is turned off). Note, in this case, the IPin variable named "outPin" is the destination buffer. The range check error occurs if range checking is turned on because Delphi treats the NIL as a zero length array.

Answer 1

I'm suspicious of your declaration of the ppPins parameter and your casts to TPointerDynArray and TIPinDynArray.

You will get range errors if you pretend that the array passed in is a Delphi dynamic array when in fact it is not. Delphi dynamic arrays have a block of memory that precedes the first item of the array and identifies the reference count to the array, and it's length. Since this block is missing in your array, tricking the compiler into believing it is there will result in the error you see.

I'd do it as shown below. This matches the declarations used in MSDN. You have to use a bit of pointer arithmetic but I believe it is easier to do this since your code now will be easy to relate to any C++ samples you can find online.

type
  PIPin = ^IPin;

function TBCEnumPins.Next(cPins: ULONG; ppPins: PIPin; pcFetched: PULONG): HRESULT;
begin
  ... 
  // this is the meat of the loop, assign to the output array, and increment
  Pointer(ppPins)^ := nil; // initialise the intf ref to avoid bogus _Release
  ppPins^ := Pin;
  inc(ppPins);
  ...
end;

Answer 2

IEnumPins::Next method is not supposed to be called with ppPins == NULL. The best way to handle is thus return immediately with E_POINTER error code.

You might want to check caller's code to find out why you have NULL there in first place.