Re: dinput: Added a check for NULL callback in EnumDevices.

[Vitaliy Margolen]

On 06/03/2011 04:19 PM, Lucas Fialho Zawacki wrote:
> In the absence of this check apps passing a NULL pointer as callback to
> EnumDevices segfault under Wine.

Do you have an example of such application? And the bug in bugzilla?

Vitaliy.

dinput: Added a check for NULL callback in EnumDevices.

[Lucas Zawacki]

> Do you have an example of such application? And the bug in bugzilla?

I don't. I just thought that a case where Wine misbehaving causes a
crash was worth fixing.

dinput: Added a check for NULL callback in EnumDevices.

[Juan Lang]

> I don't. I just thought that a case where Wine misbehaving causes a
> crash was worth fixing.

Sometimes apps depend on the crash.  Since your change comes with a
test case, it looks reasonable to me, although you might check the
specific HRESULT rather than just FAILED.
--Juan

dinput: Added a check for NULL callback in EnumDevices.

[Lucas Zawacki]

> Sometimes apps depend on the crash.  Since your change comes with a
> test case, it looks reasonable to me, although you might check the
> specific HRESULT rather than just FAILED.

Exactly, it's not a crash on Windows. I'll resend it with an explicit test.

I was thinking of this more as "janitorial" work as opposed to fixing
a buggy app, maybe my comment worded it badly.

dinput: Added a check for NULL callback in EnumDevices.

[Vitaliy Margolen]

On 06/04/2011 01:32 PM, Lucas Zawacki wrote:
> > Sometimes apps depend on the crash.  Since your change comes with a
> > test case, it looks reasonable to me, although you might check the
> > specific HRESULT rather than just FAILED.
>
> Exactly, it's not a crash on Windows. I'll resend it with an explicit test.
>
> I was thinking of this more as "janitorial" work as opposed to fixing
> a buggy app, maybe my comment worded it badly.

AJ was explicit about such checks: unless you have an application that 
breaks because of this don't add any null checks. They needlessly slowdown 
Wine. And won't guard you against invalid pointer anyway.

Vitaliy.

dinput: Added a check for NULL callback in EnumDevices.

[Lucas Zawacki]

Ok, I didnt know and that changes everything. From now on I'll keep
that in mind.

Sorry for the noise.

2011/6/4 Vitaliy Margolen :
> On 06/04/2011 01:32 PM, Lucas Zawacki wrote:
> > > Sometimes apps depend on the crash.  Since your change comes with a
> > > test case, it looks reasonable to me, although you might check the
> > > specific HRESULT rather than just FAILED.
> >
> > Exactly, it's not a crash on Windows. I'll resend it with an explicit
> > test.
> >
> > I was thinking of this more as "janitorial" work as opposed to fixing
> > a buggy app, maybe my comment worded it badly.
>
> AJ was explicit about such checks: unless you have an application that
> breaks because of this don't add any null checks. They needlessly slowdown
> Wine. And won't guard you against invalid pointer anyway.
>
> Vitaliy.