Add support for setting the log callback with libusb_set_option/libusb_init_context

This commit effectively deprecates libusb_set_log_cb by adding support for
setting the callback in either libusb_set_option or libusb_init_context. Since
the libusb_set_log_cb is already in use we can not easily deprecate it without
first incrementing the major version. We may do that in the future.

Closes #1265

Signed-off-by: Nathan Hjelm <hjelmn@google.com>

7 files changed, 273 insertions, 37 deletions

diff --git a/libusb/core.c b/libusb/core.c
index 17e6117..23ab43b 100644
--- a/libusb/core.c
+++ b/libusb/core.c
@@ -2219,6 +2219,29 @@ void API_EXPORTED libusb_set_debug(libusb_context *ctx, int level)
 #endif
 }
 
+static void libusb_set_log_cb_internal(libusb_context *ctx, libusb_log_cb cb,
+				       int mode)
+{
+#if defined(ENABLE_LOGGING) && (!defined(ENABLE_DEBUG_LOGGING) || !defined(USE_SYSTEM_LOGGING_FACILITY))
+#if !defined(USE_SYSTEM_LOGGING_FACILITY)
+	if (mode & LIBUSB_LOG_CB_GLOBAL)
+		log_handler = cb;
+#endif
+#if !defined(ENABLE_DEBUG_LOGGING)
+	if (mode & LIBUSB_LOG_CB_CONTEXT) {
+		ctx = usbi_get_context(ctx);
+		ctx->log_handler = cb;
+	}
+#else
+	UNUSED(ctx);
+#endif
+#else
+	UNUSED(ctx);
+	UNUSED(cb);
+	UNUSED(mode);
+#endif
+}
+
 /** \ingroup libusb_lib
  * Set log handler.
  *
@@ -2245,24 +2268,7 @@ void API_EXPORTED libusb_set_debug(libusb_context *ctx, int level)
 void API_EXPORTED libusb_set_log_cb(libusb_context *ctx, libusb_log_cb cb,
 	int mode)
 {
-#if defined(ENABLE_LOGGING) && (!defined(ENABLE_DEBUG_LOGGING) || !defined(USE_SYSTEM_LOGGING_FACILITY))
-#if !defined(USE_SYSTEM_LOGGING_FACILITY)
-	if (mode & LIBUSB_LOG_CB_GLOBAL)
-		log_handler = cb;
-#endif
-#if !defined(ENABLE_DEBUG_LOGGING)
-	if (mode & LIBUSB_LOG_CB_CONTEXT) {
-		ctx = usbi_get_context(ctx);
-		ctx->log_handler = cb;
-	}
-#else
-	UNUSED(ctx);
-#endif
-#else
-	UNUSED(ctx);
-	UNUSED(cb);
-	UNUSED(mode);
-#endif
+	libusb_set_log_cb_internal(ctx, cb, mode);
 }
 
 /** \ingroup libusb_lib
@@ -2292,6 +2298,7 @@ int API_EXPORTEDV libusb_set_option(libusb_context *ctx,
 	enum libusb_option option, ...)
 {
 	int arg = 0, r = LIBUSB_SUCCESS;
+	libusb_log_cb log_cb = NULL;
 	va_list ap;
 
 	va_start(ap, option);
@@ -2301,6 +2308,9 @@ int API_EXPORTEDV libusb_set_option(libusb_context *ctx,
 			r = LIBUSB_ERROR_INVALID_PARAM;
 		}
 	}
+	if (LIBUSB_OPTION_LOG_CB == option) {
+		log_cb = (libusb_log_cb) va_arg(ap, libusb_log_cb);
+	}
 	va_end(ap);
 
 	if (LIBUSB_SUCCESS != r) {
@@ -2316,12 +2326,15 @@ int API_EXPORTEDV libusb_set_option(libusb_context *ctx,
 		default_context_options[option].is_set = 1;
 		if (LIBUSB_OPTION_LOG_LEVEL == option) {
 			default_context_options[option].arg.ival = arg;
+		} else if (LIBUSB_OPTION_LOG_CB) {
+			default_context_options[option].arg.log_cbval = log_cb;
 		}
 		usbi_mutex_static_unlock(&default_context_lock);
 	}
 
 	ctx = usbi_get_context(ctx);
 	if (NULL == ctx) {
+		libusb_set_log_cb_internal(NULL, log_cb, LIBUSB_LOG_CB_GLOBAL);
 		return LIBUSB_SUCCESS;
 	}
 
@@ -2343,6 +2356,9 @@ int API_EXPORTEDV libusb_set_option(libusb_context *ctx,
 		return LIBUSB_ERROR_NOT_SUPPORTED;
 		break;
 
+	case LIBUSB_OPTION_LOG_CB:
+		libusb_set_log_cb_internal(ctx, log_cb, LIBUSB_LOG_CB_CONTEXT);
+		break;
 	default:
 		return LIBUSB_ERROR_INVALID_PARAM;
 	}
@@ -2452,14 +2468,24 @@ int API_EXPORTED libusb_init_context(libusb_context **ctx, const struct libusb_i
 		if (LIBUSB_OPTION_LOG_LEVEL == option || !default_context_options[option].is_set) {
 			continue;
 		}
-		r = libusb_set_option(_ctx, option);
+		if (LIBUSB_OPTION_LOG_CB != option) {
+			r = libusb_set_option(_ctx, option);
+		} else {
+			r = libusb_set_option(_ctx, option, default_context_options[option].arg.log_cbval);
+		}
 		if (LIBUSB_SUCCESS != r)
 			goto err_free_ctx;
 	}
 
 	/* apply any options provided by the user */
 	for (int i = 0 ; i < num_options ; ++i) {
-		r = libusb_set_option(_ctx, options[i].option, options[i].value.ival);
+		switch(options[i].option) {
+		case LIBUSB_OPTION_LOG_CB:
+			r = libusb_set_option(_ctx, options[i].option, options[i].value.log_cbval);
+			break;
+		default:
+			r = libusb_set_option(_ctx, options[i].option, options[i].value.ival);
+		}
 		if (LIBUSB_SUCCESS != r)
 			goto err_free_ctx;
 	}
diff --git a/libusb/libusb.h b/libusb/libusb.h
index 0b039d4..99cc017 100644
--- a/libusb/libusb.h
+++ b/libusb/libusb.h
@@ -3,7 +3,7 @@
  * Copyright © 2001 Johannes Erdfelt <johannes@erdfelt.com>
  * Copyright © 2007-2008 Daniel Drake <dsd@gentoo.org>
  * Copyright © 2012 Pete Batard <pete@akeo.ie>
- * Copyright © 2012-2021 Nathan Hjelm <hjelmn@cs.unm.edu>
+ * Copyright © 2012-2023 Nathan Hjelm <hjelmn@cs.unm.edu>
  * Copyright © 2014-2020 Chris Dickens <christopher.a.dickens@gmail.com>
  * For more information, please visit: http://libusb.info
  *
@@ -1534,20 +1534,18 @@ enum libusb_option {
 	 */
 	LIBUSB_OPTION_WINUSB_RAW_IO = 3,
 
-	LIBUSB_OPTION_MAX = 4
-};
+	/** Set the context log callback functon.
+	 *
+	 * Set the log callback function either on a context or globally. This
+	 * option must be provided an argument of type libusb_log_cb. Using this
+	 * option with a NULL context is equivalent to calling libusb_set_log_cb
+	 * with mode LIBUSB_LOG_CB_GLOBAL. Using it with a non-NULL context is
+	 * equivalent to calling libusb_set_log_cb with mode
+	 * LIBUSB_LOG_CB_CONTEXT.
+	 */
+	LIBUSB_OPTION_LOG_CB = 4,
 
-/** \ingroup libusb_lib
- * Structure used for setting options through \ref libusb_init_context.
- *
- */
-struct libusb_init_option {
-  /** Which option to set */
-  enum libusb_option option;
-  /** An integer value used by the option (if applicable). */
-  union {
-    int64_t ival;
-  } value;
+	LIBUSB_OPTION_MAX = 5
 };
 
 /** \ingroup libusb_lib
@@ -1564,10 +1562,25 @@ struct libusb_init_option {
 typedef void (LIBUSB_CALL *libusb_log_cb)(libusb_context *ctx,
 	enum libusb_log_level level, const char *str);
 
+/** \ingroup libusb_lib
+ * Structure used for setting options through \ref libusb_init_context.
+ *
+ */
+struct libusb_init_option {
+  /** Which option to set */
+  enum libusb_option option;
+  /** An integer value used by the option (if applicable). */
+  union {
+    int64_t ival;
+    libusb_log_cb log_cbval;
+  } value;
+};
+
 int LIBUSB_CALL libusb_init(libusb_context **ctx);
 int LIBUSB_CALL libusb_init_context(libusb_context **ctx, const struct libusb_init_option options[], int num_options);
 void LIBUSB_CALL libusb_exit(libusb_context *ctx);
 void LIBUSB_CALL libusb_set_debug(libusb_context *ctx, int level);
+/* may be deprecated in the future in favor of lubusb_init_context()+libusb_set_option() */
 void LIBUSB_CALL libusb_set_log_cb(libusb_context *ctx, libusb_log_cb cb, int mode);
 const struct libusb_version * LIBUSB_CALL libusb_get_version(void);
 int LIBUSB_CALL libusb_has_capability(uint32_t capability);
diff --git a/libusb/libusbi.h b/libusb/libusbi.h
index 72d2568..030e6b6 100644
--- a/libusb/libusbi.h
+++ b/libusb/libusbi.h
@@ -814,6 +814,7 @@ struct usbi_option {
   int is_set;
   union {
     int ival;
+    libusb_log_cb log_cbval;
   } arg;
 };
 
diff --git a/libusb/version_nano.h b/libusb/version_nano.h
index 34620df..cb1eb95 100644
--- a/libusb/version_nano.h
+++ b/libusb/version_nano.h
@@ -1 +1 @@
-#define LIBUSB_NANO 11791
+#define LIBUSB_NANO 11792
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 7831494..94ed32c 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -5,9 +5,10 @@ LIBS =
 stress_SOURCES = stress.c testlib.c
 stress_mt_SOURCES = stress_mt.c
 set_option_SOURCES = set_option.c testlib.c
+init_context_SOURCES = init_context.c testlib.c
 
 noinst_HEADERS = libusb_testlib.h
-noinst_PROGRAMS = stress stress_mt set_option
+noinst_PROGRAMS = stress stress_mt set_option init_context
 
 if BUILD_UMOCKDEV_TEST
 # NOTE: We add libumockdev-preload.so so that we can run tests in-process
diff --git a/tests/init_context.c b/tests/init_context.c
new file mode 100644
index 0000000..e7a010e
--- /dev/null
+++ b/tests/init_context.c
@@ -0,0 +1,155 @@
+/* -*- Mode: C; indent-tabs-mode:nil -*- */
+/*
+ * Unit tests for libusb_set_option
+ * Copyright © 2023 Nathan Hjelm <hjelmn@cs.unm.edu>
+ * Copyright © 2023 Google, LLC. All rights reserved.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include "config.h"
+
+#include <stdlib.h>
+#include <stdio.h>
+#include <inttypes.h>
+#include "libusbi.h"
+#include "libusb_testlib.h"
+
+#if defined(_WIN32) && !defined(__CYGWIN__)
+#include <winbase.h>
+
+static int unsetenv(const char *env) {
+  return _putenv_s(env, "");
+}
+
+static int setenv(const char *env, const char *value, int overwrite) {
+  if (getenv(env) && !overwrite)
+    return 0;
+  return _putenv_s(env, value);
+}
+#endif
+
+#define LIBUSB_TEST_CLEAN_EXIT(code) \
+  do {                               \
+    if (test_ctx != NULL) {          \
+      libusb_exit(test_ctx);         \
+    }                                \
+    unsetenv("LIBUSB_DEBUG");        \
+    return (code);                   \
+  } while (0)
+
+/**
+ * Fail the test if the expression does not evaluate to LIBUSB_SUCCESS.
+ */
+#define LIBUSB_TEST_RETURN_ON_ERROR(expr)                       \
+  do {                                                          \
+    int _result = (expr);                                       \
+    if (LIBUSB_SUCCESS != _result) {                            \
+      libusb_testlib_logf("Not success (%s) at %s:%d", #expr,   \
+                          __FILE__, __LINE__);                  \
+      LIBUSB_TEST_CLEAN_EXIT(TEST_STATUS_FAILURE);              \
+    }                                                           \
+  } while (0)
+
+/**
+ * Use relational operatator to compare two values and fail the test if the
+ * comparison is false. Intended to compare integer or pointer types.
+ *
+ * Example: LIBUSB_EXPECT(==, 0, 1) -> fail, LIBUSB_EXPECT(==, 0, 0) -> ok.
+ */
+#define LIBUSB_EXPECT(operator, lhs, rhs)                               \
+  do {                                                                  \
+    int64_t _lhs = (int64_t)(intptr_t)(lhs), _rhs = (int64_t)(intptr_t)(rhs); \
+    if (!(_lhs operator _rhs)) {                                        \
+      libusb_testlib_logf("Expected %s (%" PRId64 ") " #operator        \
+                          " %s (%" PRId64 ") at %s:%d", #lhs,           \
+                          (int64_t)(intptr_t)_lhs, #rhs,                \
+                          (int64_t)(intptr_t)_rhs, __FILE__,            \
+                          __LINE__);                                    \
+      LIBUSB_TEST_CLEAN_EXIT(TEST_STATUS_FAILURE);                      \
+    }                                                                   \
+  } while (0)
+
+
+static libusb_testlib_result test_init_context_basic(void) {
+  libusb_context *test_ctx = NULL;
+
+  /* test basic functionality */
+  LIBUSB_TEST_RETURN_ON_ERROR(libusb_init_context(&test_ctx, /*options=*/NULL,
+                                                  /*num_options=*/0));
+
+  LIBUSB_TEST_CLEAN_EXIT(TEST_STATUS_SUCCESS);
+}
+
+static libusb_testlib_result test_init_context_log_level(void) {
+  libusb_context *test_ctx = NULL;
+
+  struct libusb_init_option options[] = {
+    {
+      .option = LIBUSB_OPTION_LOG_LEVEL,
+      .value = {
+        .ival = LIBUSB_LOG_LEVEL_ERROR,
+      },
+    }
+  };
+
+  /* test basic functionality */
+  LIBUSB_TEST_RETURN_ON_ERROR(libusb_init_context(&test_ctx, options,
+                                                  /*num_options=*/1));
+
+  LIBUSB_EXPECT(==, test_ctx->debug, LIBUSB_LOG_LEVEL_ERROR);
+
+  LIBUSB_TEST_CLEAN_EXIT(TEST_STATUS_SUCCESS);
+}
+
+static void test_log_cb(libusb_context *ctx, enum libusb_log_level level,
+                        const char *str) {
+  UNUSED(ctx);
+  UNUSED(level);
+  UNUSED(str);
+}
+
+static libusb_testlib_result test_init_context_log_cb(void) {
+  libusb_context *test_ctx = NULL;
+
+  struct libusb_init_option options[] = {
+    {
+      .option = LIBUSB_OPTION_LOG_CB,
+      .value = {
+        .log_cbval = (libusb_log_cb) &test_log_cb,
+      },
+    }
+  };
+
+  /* test basic functionality */
+  LIBUSB_TEST_RETURN_ON_ERROR(libusb_init_context(&test_ctx, options,
+                                                  /*num_options=*/1));
+
+  LIBUSB_EXPECT(==, test_ctx->log_handler, test_log_cb);
+
+  LIBUSB_TEST_CLEAN_EXIT(TEST_STATUS_SUCCESS);
+}
+
+static const libusb_testlib_test tests[] = {
+  { "test_init_context_basic", &test_init_context_basic },
+  { "test_init_context_log_level", &test_init_context_log_level },
+  { "test_init_context_log_cb", &test_init_context_log_cb },
+  LIBUSB_NULL_TEST
+};
+
+int main(int argc, char *argv[])
+{
+  return libusb_testlib_run_tests(argc, argv, tests);
+}
diff --git a/tests/set_option.c b/tests/set_option.c
index c04e4ea..3c658c6 100644
--- a/tests/set_option.c
+++ b/tests/set_option.c
@@ -71,7 +71,7 @@ static int setenv(const char *env, const char *value, int overwrite) {
  */
 #define LIBUSB_EXPECT(operator, lhs, rhs)                               \
   do {                                                                  \
-    int64_t _lhs = (lhs), _rhs = (rhs);                                 \
+    int64_t _lhs = (int64_t)(intptr_t)(lhs), _rhs = (int64_t)(intptr_t)(rhs); \
     if (!(_lhs operator _rhs)) {                                        \
       libusb_testlib_logf("Expected %s (%" PRId64 ") " #operator        \
                           " %s (%" PRId64 ") at %s:%d", #lhs,           \
@@ -185,12 +185,52 @@ static libusb_testlib_result test_no_discovery(void)
 #endif
 }
 
+static void test_log_cb(libusb_context *ctx, enum libusb_log_level level,
+                        const char *str) {
+  UNUSED(ctx);
+  UNUSED(level);
+  UNUSED(str);
+}
+
+
+static libusb_testlib_result test_set_log_cb(void)
+{
+#if defined(ENABLE_LOGGING) && !defined(ENABLE_DEBUG_LOGGING)
+  libusb_context *test_ctx = NULL;
+
+  /* set the log callback on the context */
+  LIBUSB_TEST_RETURN_ON_ERROR(libusb_init_context(&test_ctx, /*options=*/NULL,
+                                                  /*num_options=*/0));
+  LIBUSB_TEST_RETURN_ON_ERROR(libusb_set_option(test_ctx, LIBUSB_OPTION_LOG_CB,
+                                                test_log_cb));
+
+  /* check that debug level came from the default */
+  LIBUSB_EXPECT(==, test_ctx->log_handler, test_log_cb);
+
+  libusb_exit(test_ctx);
+  test_ctx = NULL;
+
+  /* set the log callback for all future contexts */
+  LIBUSB_TEST_RETURN_ON_ERROR(libusb_set_option(/*ctx=*/NULL, LIBUSB_OPTION_LOG_CB,
+                                                test_log_cb));
+  LIBUSB_TEST_RETURN_ON_ERROR(libusb_init_context(&test_ctx, /*options=*/NULL,
+                                                  /*num_options=*/0));
+  LIBUSB_EXPECT(==, test_ctx->log_handler, test_log_cb);
+
+
+  LIBUSB_TEST_CLEAN_EXIT(TEST_STATUS_SUCCESS);
+#else
+  return TEST_STATUS_SKIP;
+#endif
+}
+
 static const libusb_testlib_test tests[] = {
   { "test_set_log_level_basic", &test_set_log_level_basic },
   { "test_set_log_level_env", &test_set_log_level_env },
   { "test_no_discovery", &test_no_discovery },
   /* since default options can't be unset, run this one last */
   { "test_set_log_level_default", &test_set_log_level_default },
+  { "test_set_log_cb", &test_set_log_cb },
   LIBUSB_NULL_TEST
 };