Details

Reviewers

Group Reviewers

USB
manpages

Commits

rG2879c818e553: libusb: implement libusb_set_option

Summary

Implement libusb_set_option for API compatibility of libusb upstream
The implementation status of each option is as following:

LIBUSB_OPTION_LOG_LEVEL: just like libusb_set_debug
LIBUSB_OPTION_LOG_CB: add callback support for DPRINTF
LIBUSB_OPTION_NO_DEVICE_DISCOVERY: disable initialization of devd and
netlink when register. Also, create no thread when registration of
callback happens.
LIBUSB_OPTION_USE_USBDK: no needed as USBDK is for Windows

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 65315
Build 62198: arc lint + arc unit

Event Timeline

aokblast created this revision.Jun 12 2025, 4:08 PM

Herald added a subscriber: imp. · View Herald TranscriptJun 12 2025, 4:08 PM

aokblast requested review of this revision.Jun 12 2025, 4:08 PM

Harbormaster completed remote builds in B64820: Diff 156920.Jun 12 2025, 4:08 PM

The change about DEBUG_LEVEL to LOG_LEVEL is required to allow set_debug and debug callback works. So add that patch as a parent.

Fix indent

Harbormaster completed remote builds in B64827: Diff 156935.Jun 12 2025, 7:56 PM

aokblast mentioned this in D50680: libusb: replace DEBUG_LEVEL with LOG_LEVEL.Jun 12 2025, 10:03 PM

kevans added a subscriber: kevans.Jun 12 2025, 10:17 PM

kevans added inline comments.

lib/libusb/libusb10.c
247	`log_level` seems to be unused in this final version, can it be removed?
1842	style(9) dictates explicit comparison against `NULL` here
lib/libusb/libusb10_hotplug.c
387	Is there some implication here for `no_discovery`? Should this be reachable at all if it's set?

aokblast added inline comments.Jun 13 2025, 2:38 PM

lib/libusb/libusb10_hotplug.c
387	The no_discovery in upstream libusb is utilized when init the context. It will not start any hotplug scan thread if this option is true on the upstream libusb. That means this option is useless after libusb_init is called. However, our implementation on the hotplug scan thread starts when the hotplug register is called instead of the libusb_init. I think the time point of no_discovery on upstream libusb is tighter because after init it will not work. So our implementation to do check on hotplug handler won't break any usecase of this option as it is expected to be set before init the context and actually hotplug register should be called after init.

aokblast added inline comments.Jun 13 2025, 2:39 PM

lib/libusb/libusb10.c
247	It is a miss. Sorry, I will update a newer version later.

Update style inconsistent

Harbormaster completed remote builds in B64893: Diff 157074.Jun 16 2025, 2:29 AM

Add manual

Herald added a subscriber: ziaee. · View Herald TranscriptJul 9 2025, 9:27 AM

Harbormaster completed remote builds in B65295: Diff 158211.Jul 9 2025, 9:27 AM

aokblast added a child revision: D51224: libusb: refactor the process of setting option when init the context..Jul 9 2025, 9:44 AM

Fix incorrect unlock

Harbormaster completed remote builds in B65301: Diff 158218.Jul 9 2025, 12:46 PM

Remove unneeded commit

Harbormaster completed remote builds in B65315: Diff 158250.Jul 9 2025, 11:51 PM

commit message ought to reference libusb_set_option not just add option

lib/libusb/libusb.h
261	This probably calls for a comment explaining why the value is reused (was already true before your changes)

ziaee added a reviewer: manpages.Jul 24 2025, 7:02 PM

aokblast added inline comments.Aug 5 2025, 12:46 PM

lib/libusb/libusb.h
261	The upstream code change in here: https://github.com/libusb/libusb/commit/33d9949921423d0911527e04af8372d320f2489b. They first use LIBUSB_OPTION_NO_DEVICE_DISCOVERY to replace LIBUSB_OPTION_WEAK_AUTHORITY. Then for the compatibility, they define LIBUSB_OPTION_WEAK_AUTHORITY as a macro again. I would send a different patch about this.