Nut upsdev - Dec 2008 - [nut-commits] svn commit r1596 - in trunk: drivers man tools

On Thu, Dec 4, 2008 at 4:51 PM, Arjen de Korte
<adkorte-guest at alioth.debian.org> wrote:
> Author: adkorte-guest
> Date: Thu Dec  4 21:51:28 2008
> New Revision: 1596
>
> Log:
> Added new richcomm_usb driver based on the lakeview_usb driver (using
libusb).
Sorry if I wasn't clear - I was advocating *renaming* the lakeview_usb
driver to richcomm_usb.

Is there another reason to keep both in the tree? I'm pretty sure that
this device with the Lakeview Research VID is actually made by
Richcomm, as the earlier links pointed out.

- Charles

Citeren Charles Lepple <clepple at gmail.com>:
>> Log:
>> Added new richcomm_usb driver based on the lakeview_usb driver  
>> (using libusb).
>
> Sorry if I wasn't clear - I was advocating *renaming* the lakeview_usb
> driver to richcomm_usb.
You were very clear. The reason for adding the richcomm_usb driver  
instead of replacing/renaming the existing lakeview_usb driver, is  
that the latter seems to be tested at some point in time and the first  
is not (yet).
> Is there another reason to keep both in the tree?
Yes. Although I'm fairly confident that the richcomm_usb driver will  
work, I would like to see it tested before dropping the lakeview_usb  
driver.

The reason for rewriting the lakeview_usb driver, is that it is  
duplicating lots of stuff we already have in libusb.c and also  
contains some lines that make me wonder what is meant:

55 	MESSAGE_VALUE, INDEX_VALUE, query, sizeof(query), 1000);

Here query is an argument to the call of this function of type  
(unsigned char *). On a 32-bit system this will boil down to 4, but on  
a 64-bit system most likely 8. This only works correctly if query is  
an array, which is not the case here.

62 	ret = usb_interrupt_read(upsdev, REPLY_REQUESTTYPE, reply,  
sizeof(REPLY_PACKETSIZE), 1000);

Again wrong use of sizeof(). I don't know what the preprocessor will  
make out of 'sizeof(6)', but it most likely is not what was intended.

66 	usb_comm_fail("Receive error (Request command): COMMAND: %x\n",
query);

This is a non-portable construction. The (implicit) typecasting from  
(unsigned char *) to (unsigned int) is never pretty anyway.
> I'm pretty sure that
> this device with the Lakeview Research VID is actually made by
> Richcomm, as the earlier links pointed out.
Indeed.

Best regards, Arjen
-- 
Please keep list traffic on the list