[Nut-upsdev] [nut-commits] svn commit r1074 - in trunk: .

[Arjen de Korte]

>> Maybe I'm just thick-headed, but I fail to see why the driver would
>> segfault if the information in the report descriptor is wrong. It might
>> try to read feature reports that no longer exist, but this doesn't lead
>> to segfaults (I tried this a couple of weeks ago on my Evolution 650).
>> Note that until r1080, there was a dangerous use of un-initialized data
>> that could easily lead to segfaults, so make sure you use a recent
>> version if you want to try this out.
> It's true that reading nonexistent data might not cause a segfault.
> However, reading data and misinterpreting it (because it happens to be
> in a numbered report which exists in both the current device and the
> previous one, but whose contents might be something totally different)
> can lead to a system shutdown in the worst case. Similarly, writing
> variables whose interpretation is unknown can mess up the UPS state.

I'm sorry, but this is only a theoretical problem. As I stated many times
before, the usbhid-ups driver won't support multiple UPS'es that can't be
distinguished through the regular expression matching. It never has and it
never will. As a result, I don't think it is too much to ask from people
that they need to restart the driver (manually) if they replace their UPS
by a different unit. So we can assume that when we have a match on the
exact matcher, this will be the same device as before. We should make it
much clearer in the documentation that if you want to monitor multiple
UPS'es, the regular expression matching is *not* optional, but mandatory
instead.

Ultimately, this boils down to the problem that we're trying to do add
hotplugging support to a driver, where it should really be handled by the
kernel (or HAL daemon for that matter). The hotplugging support for USB is
not in the drivers. The more I'm looking into this, the more I think the
regular expression (exact) matching is a gross hack at best. I understand
(and appreciate) the reasons how we came to this, but in the end I don't
like the result at all. It currently works reasonably well for devices
that can be uniquely identified through the regex matching mechanism, but
if this is not possible (for whatever reason) you're limited to just one
UPS on your system, which wouldn't be neccessary with hotplugging support.

I really think we'd better put our effort into adding *real* hotplugging
support. At least, that's where I'm going to put my effort in the next
couple of weeks (months). We badly need it to use the information the HAL
daemon provided about the whereabouts of the UPS, rather than trying to
detect it ourselves again (which was already done by the HAL daemon), with
the limitation that we can only support one UPS per system, because we
can't use the regular expression matching. What would be nice also, if we
would not need to make a choice between HAL and NUT support, but that the
HAL capable drivers (one dedicated one for each subdriver we have now)
would also allow monitoring by the (upsd) server.

Best regards, Arjen