[sane-devel] sanei usb improvements
Stef
stef.dev at free.fr
Fri Mar 8 05:22:07 UTC 2013
On 08/03/2013 00:45, Olaf Meeuwissen wrote:
> Stef writes:
>
>> On 05/03/2013 01:35, Olaf Meeuwissen wrote:
>>> Stef writes:
>>>
>>>> here's a patch set to improve sanei_usb to review.
>>> Patches 2, 3 and 4 are fine with me but the first patch mixes several
>>> changes that make it unnecessarily hard to review. Could split that
>>> patch so it focusses on the advertised changeset and refrain from the
>>> small changes in whitespace, comments and debugging feedback?
> Sorry for the late follow-up. I don't care for the later patches so I
> only looked at:
>
> 0001-sanei_usb_init-rework.patch
> 0002-add-sanei_usb_exit-function.patch
>
> The call to kernel_scan_devices() in sanei_usb_scan_devices() struck me
> as odd. At least for linux-2.6.3 or later it is not needed because
> there is no kernel scanner module anymore. But looking at the code, I
> noticed that it may be necessary on other OSs. Still, if using one of
> the libusb libraries on any OS, you should be fine without the kernel
> scan, no?
>
> In sanei_usb_exit() you should reset sanei_usb_ctx to NULL because you
> check for it in sanei_usb_init(). This should be fixed.
>
> As a matter of style, in sanei_usb_exit(), I would put the "int i" in
> the "if (initialized==0)" branch, limiting its existence to the scope
> where it is needed.
>
> I'll leave commenting on usbcall_scan_devices() to somebody who is
> familiar with that API (but it looks like you just lifted that from its
> original location) and I skipped all the changes you made to the
> debugging messages in the first patch (as these are off-topic).
>
> Hope this helps,
Hello,
thanks for your time and the feedback.
I thought the same about kernel scanner device. As you suggest, I
will make exclusive from libusb, so it will be used only is there no
libusb library available. I will also reset ctx to null.
Regarding the debugging message, since they are moving to another
function they had to be changed because they print the function name as
part of the message. I choose to use __FUNCTION__ so I don't have to
change them again if I put them in another function when splitting big
functions, or if I have to change the name of the enclosing function.
It's right I haven't changed the bus scanning logic, I only have
split it on bus scanning method basis. Even though there are some
'continue' and 'break' I would feel like removing.
Regards,
Stef
More information about the sane-devel
mailing list