Convert various macros to functions by seanm · Pull Request #1729 · libusb/libusb

seanm · 2025-12-31T01:31:49Z

Macros are well-known footguns. Functions provide type safety.

Youw

Can you update the PR description with the motivation for this change?

seanm · 2026-06-01T15:30:12Z

Can you update the PR description with the motivation for this change?

The motivation is the usual one when it comes to C macros: no type safety, easy footguns, etc. etc. Any compiler today will certainly inline a 1 line macro, so its only advantage is gone.

Youw · 2026-06-03T13:10:24Z

Can you update the PR description with the motivation for this change?

The motivation is the usual one when it comes to C macros: no type safety, easy footguns, etc. etc. Any compiler today will certainly inline a 1 line macro, so its only advantage is gone.

I totally get that, but I generally encourage people to be disciplined-enough to write such things down (specially in the PR description) for other developers who may have different point of view, or may try to assume things like "maybe this is a preparation for something specific, and not just a refactoring/improvement".

Youw · 2026-06-04T08:01:25Z

This highlights one more question from my side: should all these functins have a libusb-specific prefix, e.g.: handle_ctx -> libusbi_handle_ctx?
I get it gets very verbose but still, something worth discussing since we're considering renaming and already here.

seanm · 2026-06-04T19:42:18Z

This highlights one more question from my side: should all these functins have a libusb-specific prefix

Looking at core.c, many do, but not all. I'd leave that for some other PR or discussion...

mcuee · 2026-06-05T10:11:58Z

Ah, one missing file to update.
https://github.com/libusb/libusb/actions/runs/26889358753/job/79310609437?pr=1729

/src/libusb_fuzzer.cc:44:37: error: use of undeclared identifier 'LIBUSB_TRANSFER_TO_USBI_TRANSFER'
   44 |   struct usbi_transfer *itransfer = LIBUSB_TRANSFER_TO_USBI_TRANSFER(transfer);
      |                                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.

seanm · 2026-06-05T18:59:06Z

/src/libusb_fuzzer.cc:44:37: error: use of undeclared identifier 'LIBUSB_TRANSFER_TO_USBI_TRANSFER'
44 | struct usbi_transfer *itransfer = LIBUSB_TRANSFER_TO_USBI_TRANSFER(transfer);

That file is not in the libusb repo, it seems to be from here: https://github.com/google/oss-fuzz/blob/c09755737ee2d135fb003b0fb02cc79e5c7c4fdb/projects/libusb/libusb_fuzzer.cc#L21

they include libusbi.h which isn't really a public header, but of course handy to be able to fuzz more functions.

Perhaps this is an argument for not renaming these macros/functions.

mcuee · 2026-06-05T23:41:57Z

/src/libusb_fuzzer.cc:44:37: error: use of undeclared identifier 'LIBUSB_TRANSFER_TO_USBI_TRANSFER'
44 | struct usbi_transfer *itransfer = LIBUSB_TRANSFER_TO_USBI_TRANSFER(transfer);

That file is not in the libusb repo, it seems to be from here: https://github.com/google/oss-fuzz/blob/c09755737ee2d135fb003b0fb02cc79e5c7c4fdb/projects/libusb/libusb_fuzzer.cc#L21

they include libusbi.h which isn't really a public header, but of course handy to be able to fuzz more functions.

I see.

Perhaps this is an argument for not renaming these macros/functions.

One possibility is that we merge this PR first and then ask oss-fuzz to update.

But then there may be other such cases, so in the end we may have to keep the namaes of such macros/functions.

mcuee · 2026-06-05T23:43:47Z

Let's hear from @sonatique and @tormodvolden as well on this topic.

Youw · 2026-06-06T07:26:41Z

But then there may be other such cases, so in the end we may have to keep the namaes of such macros/functions.

We could have functions with new names, and have macros as aliases for functions. This way we don't use UPPER_CAMEL_CASE for function names (directly) and still have backwarc compatibility.

sonatique · 2026-07-13T23:25:49Z

Thanks @seanm : I went through the full diff and the conversion is faithful (the one semantic change, transfer_priv_to_usbi_transfer using usbi_backend.transfer_priv_size instead of sizeof(*transfer_priv), is equivalent at its only call site and matches the allocation math in io.c). A few things before this can merge:

Rebase needed : the branch conflicts with master (core.c, windows_winusb.c moved).
CIFuzz failure is caused by this PR : the OSS-Fuzz harness (libusb_fuzzer.cc in google/oss-fuzz) still uses LIBUSB_TRANSFER_TO_USBI_TRANSFER.
macOS job needs a green rerun (likely CI flakiness, not this PR, but we need a passing run on the current head).
Nit: new function signatures use inconsistent spacing (is_ep_in (uint8_t ep), type * name) , please match the codebase's type *name(args) style.

seanm · 2026-07-14T02:43:18Z

Fixed
I think we should just not rename these things, to keep source compatibility. i.e. drop the last commit.
Agreed
Let's decide about no. 2 first, because rebasing that will be a bit of a PITA. Also: expending brain cells on this when we have clang-format is a waste of time. We should try to push Agree on formatting style, and then create .clang-format config file to enforce it #1442 again some day soon...

sonatique · 2026-07-14T15:35:41Z

@seanm : I concur with"not rename" for point 2.

- For USBI_TRANSFER_TO_TRANSFER_PRIV use void* as the return type since indeed the struct is different for each backend, and not a char* - For USBI_TRANSFER_TO_LIBUSB_TRANSFER, use the size from usbi_backend.transfer_priv_size, just like the inverse macro does. This makes things symetric and makes it possible to convert this to a function since `struct usbi_transfer` is not a macro parameter, but just resolved after preprocessing

Only 1 caller needed to be changed, since it was not acting as a function. Functions provide type safety vs macros, and macros are renowned footguns.

seanm · 2026-07-14T17:40:36Z

@seanm : I concur with"not rename" for point 2.

Done.

And I think I've done 4 too.

Youw · 2026-07-15T15:53:22Z

I think strictly correct and compatible way would be to do: seanm#1

mcuee added the core Related to common codes label Jan 2, 2026

seanm requested review from sonatique and tormodvolden January 9, 2026 02:25

mcuee added this to the 1.0.31 milestone Apr 13, 2026

mcuee mentioned this pull request May 6, 2026

Misc: To release 1.0.31 #1806

Open

seanm requested a review from Youw May 31, 2026 20:58

Youw reviewed Jun 1, 2026

View reviewed changes

Comment thread libusb/libusbi.h Outdated

seanm force-pushed the macros-functions branch from e7b726b to 03da51d Compare June 1, 2026 16:01

seanm mentioned this pull request Jun 3, 2026

deprecate LIBUSB_NANO; use libusb_version::describe instead #1839

Merged

seanm force-pushed the macros-functions branch from 03da51d to 688cc1c Compare June 3, 2026 13:53

mcuee approved these changes Jun 5, 2026

View reviewed changes

seanm force-pushed the macros-functions branch from 688cc1c to f4ebe69 Compare July 7, 2026 20:20

seanm force-pushed the macros-functions branch from f4ebe69 to 3988608 Compare July 14, 2026 02:27

Move some macros within the file, no change to macro definitions

16aaabc

seanm force-pushed the macros-functions branch from 3988608 to 89df0b8 Compare July 14, 2026 17:34

seanm force-pushed the macros-functions branch 2 times, most recently from cb3adc8 to a75700d Compare July 14, 2026 17:38

Convert some function-like macros to real functions

a6f9da6

Only 1 caller needed to be changed, since it was not acting as a function. Functions provide type safety vs macros, and macros are renowned footguns.

seanm force-pushed the macros-functions branch from a75700d to a6f9da6 Compare July 14, 2026 17:40

Youw mentioned this pull request Jul 15, 2026

Use usbi_ helper functions behind macro aliases seanm/libusb#1

Open

Uh oh!

Conversation

seanm commented Dec 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Youw left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

seanm commented Jun 1, 2026

Uh oh!

Youw commented Jun 3, 2026

Uh oh!

Youw commented Jun 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

seanm commented Jun 4, 2026

Uh oh!

mcuee commented Jun 5, 2026

Uh oh!

seanm commented Jun 5, 2026

Uh oh!

mcuee commented Jun 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mcuee commented Jun 5, 2026

Uh oh!

Youw commented Jun 6, 2026

Uh oh!

sonatique commented Jul 13, 2026

Uh oh!

seanm commented Jul 14, 2026

Uh oh!

sonatique commented Jul 14, 2026

Uh oh!

seanm commented Jul 14, 2026

Uh oh!

Youw commented Jul 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

seanm commented Dec 31, 2025 •

edited

Loading

Youw commented Jun 4, 2026 •

edited

Loading

mcuee commented Jun 5, 2026 •

edited

Loading