[PATCH] Device init: Don't crash when CreateGC fails.

[Peter Hutterer]

On Fri, May 14, 2010 at 12:47:06PM -0700, Jamey Sharp wrote:
> On Fri, May 14, 2010 at 12:40 PM, Jeremy Huddleston <jeremyhu at apple.com> wrote:
> > Thanks Jamey!
> >
> > Reviewed-by: Jeremy Huddleston <jeremyhu at apple.com>
> 
> No problem! Out of curiousity, did you just review the patch, or also
> test it? I'd be inclined to add a tested-by line as well for you if
> you did, in fact, test. :-)
> 
> Also I'm really hoping Peter or somebody else who knows the input bits
> will review this before it's merged. I'm confident it fixes the
> symptom, and I'm pretty sure the patch isn't outright wrong, but there
> might be something better to do?

The calloc part is certainly correct, the other hunk is problematic. If you
return here, the device is marked as initialized but no presence event is
sent. Then, when the device is removed, a DeviceRemoved event will be sent
for a device that was never visible on the protocol.
Note that clients must be able to handle this anyway because given the right
timing, this could happen under normal circumstances.

However to stay symmetrical in the server, the alternative would be just do 
ret = pScreen->DeviceCursorInitialize(dev, pScreen);

This way we send out the DeviceAdded event (which makes sense, given that
we've also asked the driver to initalize everything) and then rely on the
caller of ActivateDevice to call RemoveDevice for cleanup to undo the
damage. I think a BadAlloc would be better to return code than
BadImplementation though. This path is called when creating new master
devices and there's no reason they couldn't fail for various reasons with
the server happily continuing to run. 

(Of course, that would need the matching checks in Xi/xichangehierarchy.c,
where we just ignore the ActivateDevice return values and pretend life is
perfect. That'd be a follow-up patch though.)

so, given that change
Reviewed-by: Peter Hutterer <peter.hutterer at who-t.net>

and one nitpick - please use spaces here, ActivateDevice is (surprisingly)
consistent with spaces-only indentation.

Cheers,
  Peter