ts_close() calls ops->fini() unconditionally

[ahartmetz]

I am using tslib (1.1.0, what Toradex ships in their BSP...) with a custom input driver that has a null fini "vtable" entry, like a fair number of other drivers. The window system is X because the hardware, Tegra 2, requires that. When the last X application closes, X calls ts_close(), which crashes. gdb shows that the instruction pointer is 0x0 when the crash happens. I think what happens is that the null function pointer for fini is called.
This seems fishy because it should be too obvious a bug to have persisted that long. Or is it just that ts_close() is rarely used?

[merge]

Hi,

Thanks for reporting this. I fixed similar issues recently; and it is kind of fishy indeed, but makes sense and will of course be fixes for 1.10.

I have the impression that people really badly needed tslib back in the day, took it as it is, working around all it's issues and didn't bother fixing it.

[ahartmetz]

Thanks for the quick reply. I will work around it by adding a fini function for now. By "fishy" I was referring to my analysis - it seemed unlikely that I'd be the first one to find such an -in retrospect- obvious bug.

[merge]

Totally. I had quite some of these moments since I'm working on tslib :) In that regard it's a funny project.

Sidenote: Are you allowed to share the plugin of the BSP you are using?

[merge]

If you want to give me your name and email address, I'd add you to the THANKS file.

[ahartmetz]

Maybe ts_close() should free() the tslib_module_info when the destructor is null. The modules that have destructors do that in their destructor.

The driver I've written is for a controller called called DUS3000 from DMC company, Japan. It has a self-calibration feature and our (customer's) hardware has a known screen size so I've just hardcoded the screen size and omit the tslib calibration module. The self-calibration (whose data is saved in flash so it doesn't need to run on every startup) needs to be triggered somehow, which I've done with an environment variable.
The DUS3000 hardware is also used through its serial interface in the current device. For simplicity I'm using the single-touch protocol - there is also a multi-touch protocol. The USB interface seems to offer some kind of "Windows standard" protocol which can probably be used through a standard driver in the Linux kernel.
All that limits the general usefulness of the driver. If you still want to add it to tslib, I can probably get approval to give you the code. The data sheet is freely redistributable so I could pass that along as well.

For the credits, my name is Andreas Hartmetz and my e-mail address is ahartmetz@gmail.com.

[merge]

Maybe ts_close() should free() the tslib_module_info when the destructor is null. The modules that have destructors do that in their destructor.

Yes, true. I'll have a look. Either doing it in ts_close() or adding some destructors to the modules would make sense.

[merge]

All that limits the general usefulness of the driver. If you still want to add it to tslib, I can probably get approval to give you the code. The data sheet is freely redistributable so I could pass that along as well.

It would be interesting, how compatible or different it is to the current dmc-raw.c (module_raw dmc) we have.

[ahartmetz]

It seems pretty different.
I got the permission to give you the driver, but of course I can't make improvements on it that the customer doesn't need on customer time - and I have enough side projects already. So how should I pass you the driver? I guess either a pull request that you amend somehow or I can also just mail it to you.

[merge]

Could well be that it's too different and I'll just drop it. But if it's no problem, just send it, martink@posteo.de and I'll see what to do later.

thanks

[merge]

I included it. just fyi. Since there's really no support in Linux, why not. the linear filter, and using ts_calibrate is needed for calibration in contrast to your solution, to make it more generic.

That said, the real fix would be to add support for it to the kernel - but I don't have the device, so... but at least for the usb interface it would be quite simple to do.

Thanks a lot for sharing!