a) An effectSpecific structure corresponding with a condition effect, can have 2 possible valid sizes, instead of 1: sizeof(DICONDITION) and 2*sizeof(DICONDITION); the latter is even more common. Solution: change in dinput/joystick.c:216 : if (eff->cbTypeSpecificParams != sizeof(DICONDITION)) { WARN("Effect claims to be a condition but the type-specific params are the wrong size!\n"); } else { _dump_DICONDITION(eff->lpvTypeSpecificParams); } to : if (eff->cbTypeSpecificParams == sizeof(DICONDITION)) { _dump_DICONDITION(eff->lpvTypeSpecificParams); } else if (eff->cbTypeSpecificParams == 2 * sizeof(DICONDITION)) { TRACE("Effect has two condition blocks:\n"); _dump_DICONDITION(eff->lpvTypeSpecificParams); _dump_DICONDITION(eff->lpvTypeSpecificParams + sizeof(DICONDITION)); } else { WARN("Effect claims to be a condition but the type-specific params are the wrong size (0x%x instead of 0x%x)!\n", eff->cbTypeSpecificParams, sizeof(DICONDITION)); }
b) Question: consider following code in dinput/effect_linuxinput:369 : if ((dwFlags & ~DIEP_NORESTART & ~DIEP_NODOWNLOAD & ~DIEP_START) == 0) { /* set everything */ dwFlags = DIEP_AXES | DIEP_DIRECTION | DIEP_DURATION | DIEP_ENVELOPE | DIEP_GAIN | DIEP_SAMPLEPERIOD | DIEP_STARTDELAY | DIEP_TRIGGERBUTTON | DIEP_TRIGGERREPEATINTERVAL | DIEP_TYPESPECIFICPARAMS;
Q1: Why is everything set, if dwFlags has no bits set (except DIEP_NORESTART, DIEP_NODOWNLOAD or DIEP_START)? I can see no mention of it on MSDN.
Q2: Why "dwFlags = " and not "dwFlags |= "? E.g. assume dwFlags = DIEP_START, then the if-test will cause the DIEP_START flag to be lost, resulting in an effect not started.
c) If Q1 is right, then the order of "dump_DIEFFECT" on dinput/effect_linuxinput:367 should be changed, from : dump_DIEFFECT(peff, &This->guid, dwFlags);
if ((dwFlags & ~DIEP_NORESTART & ~DIEP_NODOWNLOAD & ~DIEP_START) == 0) { /* set everything */ dwFlags = DIEP_AXES | DIEP_DIRECTION | DIEP_DURATION | DIEP_ENVELOPE | DIEP_GAIN | DIEP_SAMPLEPERIOD | DIEP_STARTDELAY | DIEP_TRIGGERBUTTON | DIEP_TRIGGERREPEATINTERVAL | DIEP_TYPESPECIFICPARAMS; } to : if ((dwFlags & ~DIEP_NORESTART & ~DIEP_NODOWNLOAD & ~DIEP_START) == 0) { /* set everything */ dwFlags = DIEP_AXES | DIEP_DIRECTION | DIEP_DURATION | DIEP_ENVELOPE | DIEP_GAIN | DIEP_SAMPLEPERIOD | DIEP_STARTDELAY | DIEP_TRIGGERBUTTON | DIEP_TRIGGERREPEATINTERVAL | DIEP_TYPESPECIFICPARAMS; }
dump_DIEFFECT(peff, &This->guid, dwFlags);
d) Add debug info for rglDirection: Change in dinput/joystick.c:169 : TRACE(" - dwTriggerButton: %d\n", eff->dwTriggerButton); TRACE(" - dwTriggerRepeatInterval: %d\n", eff->dwTriggerRepeatInterval); TRACE(" - rglDirection: %p\n", eff->rglDirection); TRACE(" - cbTypeSpecificParams: %d\n", eff->cbTypeSpecificParams); TRACE(" - lpvTypeSpecificParams: %p\n", eff->lpvTypeSpecificParams);
/* Only trace some members if dwFlags indicates they have data */ if (dwFlags & DIEP_AXES) { TRACE(" - cAxes: %d\n", eff->cAxes); TRACE(" - rgdwAxes: %p\n", eff->rgdwAxes);
if (TRACE_ON(dinput) && eff->rgdwAxes) { TRACE(" "); for (i = 0; i < eff->cAxes; ++i) TRACE("%d ", eff->rgdwAxes[i]); TRACE("\n"); } } to : TRACE(" - dwTriggerButton: %d\n", eff->dwTriggerButton); TRACE(" - dwTriggerRepeatInterval: %d\n", eff->dwTriggerRepeatInterval); TRACE(" - cbTypeSpecificParams: %d\n", eff->cbTypeSpecificParams); TRACE(" - lpvTypeSpecificParams: %p\n", eff->lpvTypeSpecificParams);
/* Only trace some members if dwFlags indicates they have data */ if ((dwFlags & DIEP_AXES) || (dwFlags & DIEP_DIRECTION)) { TRACE(" - cAxes: %d\n", eff->cAxes);
if (dwFlags & DIEP_AXES) { TRACE(" - rgdwAxes: %p\n", eff->rgdwAxes);
if (TRACE_ON(dinput) && eff->rgdwAxes) { TRACE(" "); for (i = 0; i < eff->cAxes; ++i) TRACE("%d ", eff->rgdwAxes[i]); TRACE("\n"); } }
if (dwFlags & DIEP_DIRECTION) { TRACE(" - rglDirection: %p\n", eff->rglDirection);
if (TRACE_ON(dinput) && eff->rglDirection) { TRACE(" "); for (i = 0; i < eff->cAxes; ++i) TRACE("%d ", eff->rglDirection[i]); TRACE("\n"); } } }
e) Add debug info for dwSamplePeriod: Change in dinput/joystick.c:166 : if (eff->dwGain > 10000) WARN("dwGain is out of range (>10,000)\n");
TRACE(" - dwTriggerButton: %d\n", eff->dwTriggerButton); to : if (eff->dwGain > 10000) WARN("dwGain is out of range (>10,000)\n");
if (dwFlags & DIEP_SAMPLEPERIOD) TRACE(" - dwSamplePeriod: %d\n", eff->dwSamplePeriod); TRACE(" - dwTriggerButton: %d\n", eff->dwTriggerButton);
f) Why not consistently only trace set (and probably thus valid) parameters in dump_DIEFFECT?
Change in dinput/joystick.c:163 : TRACE(" - dwDuration: %d\n", eff->dwDuration); TRACE(" - dwGain: %d\n", eff->dwGain);
if (eff->dwGain > 10000) WARN("dwGain is out of range (>10,000)\n");
if (dwFlags & DIEP_SAMPLEPERIOD) TRACE(" - dwSamplePeriod: %d\n", eff->dwSamplePeriod); TRACE(" - dwTriggerButton: %d\n", eff->dwTriggerButton); TRACE(" - dwTriggerRepeatInterval: %d\n", eff->dwTriggerRepeatInterval); TRACE(" - cbTypeSpecificParams: %d\n", eff->cbTypeSpecificParams); TRACE(" - lpvTypeSpecificParams: %p\n", eff->lpvTypeSpecificParams); to : if (dwFlags & DIEP_DURATION) TRACE(" - dwDuration: %d\n", eff->dwDuration); if (dwFlags & DIEP_GAIN) { TRACE(" - dwGain: %d\n", eff->dwGain);
if (eff->dwGain > 10000) WARN("dwGain is out of range (>10,000)\n"); }
if (dwFlags & DIEP_SAMPLEPERIOD) TRACE(" - dwSamplePeriod: %d\n", eff->dwSamplePeriod); if (dwFlags & DIEP_TRIGGERBUTTON) TRACE(" - dwTriggerButton: %d\n", eff->dwTriggerButton); if (dwFlags & DIEP_TRIGGERREPEATINTERVAL) TRACE(" - dwTriggerRepeatInterval: %d\n", eff->dwTriggerRepeatInterval); if (dwFlags & DIEP_TYPESPECIFICPARAMS) { TRACE(" - cbTypeSpecificParams: %d\n", eff->cbTypeSpecificParams); TRACE(" - lpvTypeSpecificParams: %p\n", eff->lpvTypeSpecificParams); }
and change on dinput/joystick.c:194 : if (eff->dwSize > sizeof(DIEFFECT_DX5)) TRACE(" - dwStartDelay: %d\n", eff->dwStartDelay); to : if ((dwFlags & DIEP_STARTDELAY) && (eff->dwSize > sizeof(DIEFFECT_DX5))) TRACE(" - dwStartDelay: %d\n", eff->dwStartDelay);
Note: If applying that patch, maybe we should use TRACE_ON(dinput) to circumvent all these ifs for performance reasons?
Elias
-
Elias Vanderstuyft