Is this just general unused variable cleanup? I tend to do these small unrelated cleanups in separate patches. This way, the small cleanup gets its own commit and stays if your bigger patch needs to be reverted for whatever reason.

In the rest of your patch, you opened usbd_fd as a normal descriptor with O_RDONLY and now you're opening it as a directory descriptor. Maybe the rest of the usbd_fd open() calls should be updated to use O_PATH | O_DIRECTORY and I think you could eliminate most of the capabilities in your rights list for usbd_fd since I only see you using it for openat().

The fd returns from the openat will inherit all of the capabilities from the fd pass to openat. Therefore, for the /dev/usb/*.*.* devices, we should give the usbd_fd, READ, WRITE, EVENT, IOCTL for later usage.

I'd add a .Xr capsicum 4 here since you're mentioning it earlier.

These extern declarations aren't really doing anything since you declare both variables right below.

The extern declarations in dump.c should be enough. You could move them into a header and include that if you anticipate other source files needing to access them as well.

Oops, you're definitely right. Looks good to me then :)

You are right, I am too lazy:). Fix it now.

I'd maybe re-word this slightly to something like:

Prepare
.Nm
for operation within a sandbox.
Resources that will be needed for regular operation are pre-opened
with appropriate
.Xr rights 4
applied.
Note that it is users' responsiblity to call
.Fn cap_enter .

This implies a little more heavily that libusb is opening persistent resources.

I'd maybe name it a more generic LIBUSB_OPTIOIN_SANDBOX; the libusb that everyone else uses could have platform-specific bits that are required to enable sandboxing with it.

Why do we need the caller to explicitly ask the library to be capsicum-safe? Can't it just always use capsicum-safe operations? During library initialization, have libusb open whichever /dev nodes it needs, and keep the descriptors saved in some context structure. Applications which use libusb shouldn't need to change.

Suppose these fields are already set, i.e., LIBUSB_OPTION_CAPSICUMIZE was configured once already. Then the second time, the descriptors will be leaked.

Same comment as below about cleaning up.

The return value should be checked, see my comment below.

Shouldn't the library tell you which rights are needed? The application doesn't really know, it's just treating the usb descriptors as an opaque handle.

This is main(), so when you return, the program exits. So, I don't think it's necessary to have all of this cleanup code to close file descriptors.

• This should check for errors (the main one being that the kernel can be compiled without capsicum support).
• Better to use caph_enter().

I think you need capiscum_helpers.h for this.

dump_device_info()->_device_desc()->load_vendors() will try to open _PATH_LUSBVDB, and that'll fail in capability mode.

I think this is straightforward to deal with, just make sure usb_vendors is initialized before you enter capability mode.

There is some code under share/examples/libusb20 which uses these interfaces, it should be updated too.

Why is it okay to change the library ABI?

Yes, we definately needs to merge ABI tagging back to fix it

I think we should just switch to the new interface at the same time that we bump the so version.

I uses 1.9 originally as I see it is our latest symbols version. I am also not familier with our rule. Also, I have two questions now.

Is it too late to overwrite with 1.8? My patch for versioning libusb is landed earlier but I am not sure if it can be revoked?

If we want to change the symbol versioning. Would it be great to use libusb's own versioning? LUSB_1.0 or something like this. I see some libraries have their own versioning namespace.

I try to not change to much thing at the same time but you are right:). Wil be reflect in my final version.

Oh yes. Thanks for pointing this out. I will remove this.

Oh, yeah, it's not really important to provide compat here. Suppose this patch is committed today. Then, anyone who compiled libusb applications against main in the past 3 days will get backwards compat. It is not worth bothering.

Sorry, I didn't mean you should fix it in this patch, it's fine to defer it. Just something I noticed while reading.

I think we have some leeway in main to make such a change if little time has elapsed; we'd just send a heads-up to the -current mailing list. But this is a bit of an odd case and I'd like to see what @kib suggests.

We do need a place were we can fix ABI bugs., So yes, generally as far as something is not yet in a stable branch, we can tweak it.

WRT using the libusb-own schema for versioning namespaces, it would just cause anybody working on it to ask a guidance what to do there. IMO it is better to stuck with the common approach if technically possible.

Isn't this if() pointless?

Why it does not use __BEGIN_DECLS etc?

Will fix it in another patch. Thanks!