Re: [PATCH 1/4] hid-multitouch: Auto detection of maxcontacts

[Henrik Rydberg]

> > if (td->mtclass->maxcontacts > td->maxcontacts)
> >
> >> +                     /* check if the maxcontacts is given by the class */
> >> +                     td->maxcontacts = td->mtclass->maxcontacts;
> >> +
> >> +             if (!td->maxcontacts)
> >> +                     td->maxcontacts = MT_CONTACTMAX_DEFAULT;
> >
> > this part can be then dropped
> 
> Well, it works the way you are suggesting. BTW this let the corner
> case where someone adds a device (MT_CLS) that does not send the
> contact max and does not initialize the .maxcontact field.

Yes, the patch changes the current, perfectly reasonable assumption
that maxcontact is set. If that change is removed from the patch, it
makes the logic simpler, without changing the semantics of the current
code.

> >> +     td->slots = kzalloc(td->maxcontacts * sizeof(struct mt_slot),
> >> +                             GFP_KERNEL);
> >> +
> >
> > Don't we have a race problem here?  It seems the device is started at
> > this point, so I worry that events will be handled when slots is still
> > NULL.
> 
> I tried again yesterday: if I put this line above the hid_hw_start ->
> kernel oops at first touch.
> The point is that hid_hw_start calls hid_connect that do the actual
> calls to input_mapping and feature_mapping.

Yes, that is clear, but the urbs are running, and as fas as I can see,
irqs can be delivered to mt_event(). Minute time window, testing is
not likely to hit this. Adding a test for completed initialization in
mt_event() would make sure.

Oh, and there is no test for failed memory allocation.

Thanks.
Henrik
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/