[sane-devel] sanei usb improvements
olaf.meeuwissen at avasys.jp
Fri Mar 8 09:01:03 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:
>> 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,
> 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.
OK, I see. I missed this little detail during my review where I tried
to focus on the logic. But of course you're right that the debugging
feedback needs to be adjusted. It's just that I would have done that in
a separate changeset, no big deal. Please note that __FUNCTION__ is not
part of any C standard, but __func__ is in C99. Hmm, both are used
in the backends. A majority (of files) vote would favour __FUNCTION__.
I guess either way is fine.
Hope this helps,
Olaf Meeuwissen, LPIC-2 FLOSS Engineer -- AVASYS CORPORATION
FSF Associate Member #1962 Help support software freedom
More information about the sane-devel