libusb: replace DEBUG_LEVEL with LOG_LEVEL

[aokblast]

Summary

The function "libusb_set_debug" s et level level by libusb_log_level instead of
libusb_debug_level. To be batter compatible with libusb interface.
We use the libusb_log_level as the original libusb used.

Also, delete the debug_level enum to force maintainer upgrade from their
legacy source code.

[aokblast]

Use >= instead of or

[bz]

DPRINTF usually is a FreeBSD specific debugging printf which needs to be enabled at compile time (not in this case) and is purely for developers. It is not uncommon to have different flags depending on what part to debug.

I do not fully understand the reasoning that prompted this change? Can you elaborate a bit more why this change is needed?

[aokblast]

I am fixing https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=277653.

In D50680#1156990, @bz wrote:

DPRINTF usually is a FreeBSD specific debugging printf which needs to be enabled at compile time (not in this case) and is purely for developers. It is not uncommon to have different flags depending on what part to debug.

I do not fully understand the reasoning that prompted this change? Can you elaborate a bit more why this change is needed?

The libusb provides libusb_set_debug to decide which level is the debug output.

See: https://github.com/libusb/libusb/blob/15a7ebb4d426c5ce196684347d2b7cafad862626/libusb/core.c#L2213

However, the meaning of "level" in FreeBSD libusb is different from the original implementation. The original implementation indicate the critical level on the message instead of the FreeBSD one, which indicates the source where the error emit.

This cause the application would like to set debug level (like stlink) unable to use the correct level because they do not have the same meaning.

I think this is a API incompatible between these two different version of libusb.

Also, it seems that the DPRINTF in FreeBSD libusb is not a compile time macro as

		if ((ctx)->debug & LIBUSB_DEBUG_FUNCTION) {	\
			printf("LIBUSB_FUNCTION: "		\
			       format "\n", ## __VA_ARGS__);	\
		}

ctx->debug can be set at runtime by libusb_set_debug.

[aokblast]

Replace implementation as a function call to prepare support for log callback

[aokblast]

Use vsprintf as callback request a string instead of a fmt + va_args

[aokblast]

Fix indent

[aokblast]

Output to stdout

[aokblast]

• libusb: add option

[aokblast]

Rebase to a correct version

[kevans]

lib/libusb/libusb10.c

1833

va_start should move to after the ctx->debug check; I think this is technically UB as-is to have a return-path that doesn't va_end it, and we don't really need to start it this early.

1840

Is the round-trip through buffer and using fputs entirely because you get warnings/errors about format strings not being a literal when you try to use vfprintf with new_fmt?

lib/libusb/libusb10.h

52

This should likely be tagged with __printflike(3, 4) from <sys/cdefs.h>

[aokblast]

Fix comment's error and rebase to main

[aokblast]

lib/libusb/libusb10.c

1840

This is needed because libusb upstream provide a option to overwrite the log with user defined callback. In the prototype of callback, it accept a string instead of a va_args. So we should build the pure string before we put into the callback function.

See: https://reviews.freebsd.org/D50818

[kevans]

lib/libusb/libusb10.c

1840

Ahh, ok, that's incredibly helpful context, thanks!

[aokblast]

@bz Is there still any concern about this patch? @bapt and what is your opinion about this?
If there is no any problem, I will prepare a github link for merging.

[aokblast]

Please use this patch to merge. Thanks! https://github.com/aokblast/freebsd-src/commit/e404814eed946bcd6551d163dbfeeeb951bf935b.patch

[aokblast]

fix format

[markj]

Approved, but see the inline comment.

lib/libusb/libusb10.h

50–54

Unrelated whitespace change?

[aokblast]

Fix whitespace

[lwhsu]

Just checking the metadata on https://github.com/aokblast/freebsd-src/commit/e404814eed946bcd6551d163dbfeeeb951bf935b.patch , some nits:

• Check your Author line matches your Commit line
• In convention we'll have a empty line before the Reviewed by (and don't forget to add markj)
• To be batter compatible with libusb interface, We use libusb_log_level as the original libusb does. -> we use ...
• The upstream libusb has a option to allow user to overwrite -> has an option, allows users to, and I guess to override might be more suitable here.
• do all va_arg parser inside -> parsing