[RFC 1/11] Linux FF: Joystick control panel program

[Elias Vanderstuyft]

On Mon, Mar 10, 2014 at 4:47 PM, Andrew Eikum <aeikum at codeweavers.com> wrote:
>
> On Fri, Mar 07, 2014 at 10:01:30PM +0100, Elias Vanderstuyft wrote:
> > On Wed, Mar 5, 2014 at 3:49 PM, Andrew Eikum <aeikum at codeweavers.com> wrote:
> > > On Tue, Mar 04, 2014 at 08:19:16PM +0100, Elias Vanderstuyft wrote:
> > >> a)  When unloading conditional effects in joy.cpl, the program crashes:
> > >>         When unloading the conditional effect array entries,
> > >>         Wine tries to access an invalid (high) address because
> > >>         dieffect.lpvTypeSpecificParams was not set at the time the
> > >> effect was created.
> > >>         So declare a valid 'standard' (maxed out) conditional effect.
> > >>         For other unknown effects:
> > >>             - The same can happen when CustomForce effects are available,
> > >>                 but because Wine's FF dinput->linux translation does
> > >> not support this yet,
> > >>                 we simply print a FIXME to indicate a possible cause
> > >> if a crash would happen.
> > >>             - The same for other (unsupported) effects (this is
> > >> probably not safe?),
> > >>                 but then we print a WARN.
> > >
> > > For unhandled types, it's probably best to print a FIXME and return
> > > DIENUM_CONTINUE, rather than continue to rely on uninitialized data.
> >
> > Alright.
> > Should I also write the extra if-test for the CustomForce type, or do
> > I have to cover it with one if-test as an unhandled type?
> >
>
> I'd just treat it as an unhandled type.

OK.

> > Before I knew the joy.cpl app was to be blamed, and was searching in
> > the IDirectInputEffect implementions, I saw something that is not
> > conform MSDN:
> > JoystickAImpl_EnumEffects and JoystickWImpl_EnumEffects should check
> > on DIENUM_CONTINUE to continue the enumeration, or DIENUM_STOP to stop
> > it:
> >     http://msdn.microsoft.com/en-us/library/windows/desktop/microsoft.directx_sdk.callback.dienumeffectscallback%28v=vs.85%29.aspx
> > For example, in JoystickWImpl_EnumEffects(), each occurrence of
> > "(*lpCallback)(&dei, pvRef);" should check whether the return value is
> > DIENUM_CONTINUE or DIENUM_STOP. When the latter is the case, should we
> > then first add a "goto" statement to jump to the last "if (xfd == -1)
> > IDirectInputDevice8_Unacquire(iface);" statement, or directly return?
> > And what should it return in that case, "DIERR_NOTINITIALIZED" (does
> > stopping enumeration mean failure? that's not clear in MSDN), or
> > "DI_OK"?
>
> Probably the goto. I'm not sure about the return value, you would need
> tests to be certain. Probably DI_OK is correct.

OK,
I'll queue all patches that need testing (and open a bug report if I
really don't have enough time to create the tests by myself.)
For the other patches, I'll post them as soon as possible to wine-patches.

>
> > And even worse, what if the return value of "(*lpCallback)(&dei,
> > pvRef);" is not equal to DIENUM_CONTINUE or DIENUM_STOP? Should
> > JoystickWImpl_EnumEffects() interpret it as the former or as the
> > latter? (also not mentioned in MSDN)
> >
>
> Again, you'd need tests to confirm.

OK.

>
> >         dieffect.cAxes = 2;
> >         dieffect.rgdwAxes = axes;
> >         dieffect.rglDirection = direction;
> >
> >         DIRAMPFORCE rforce;
> >         DICONSTANTFORCE cforce;
> >         DIPERIODIC pforce;
> >         DICONDITION cxyforce[2];
> >
> >         if (IsEqualGUID(&pdei->guid, &GUID_RampForce))
> >
>
> Yes, but the declarations have to be at the very top of the function,
> alongside all of the other declarations.

Oh, thanks for noticing, I should have known that ;)

Elias