Skip to content

Add missing dependency tusb.h for tusb_private.h#1977

Closed
JayToltTech wants to merge 1 commit into
hathach:masterfrom
JayToltTech:master
Closed

Add missing dependency tusb.h for tusb_private.h#1977
JayToltTech wants to merge 1 commit into
hathach:masterfrom
JayToltTech:master

Conversation

@JayToltTech
Copy link
Copy Markdown
Contributor

Describe the PR
When working with TinyUSB, my IDE (VSCode) sorted the #includes for tusb_private.h as part of automated code formatting which revealed an implicit dependency on tusb.h via build breaks when tusb_private.h is #included without/before tusb.h.

This PR simply adds the implicit dependency as an explicit dependency, fixing the build break if the #include statements are ordered differently (for example in tusb.c).

Additional context
VSCode auto-formatting changed tusb.c from:

#include "tusb.h"
#include "common/tusb_private.h"

to

#include "common/tusb_private.h"
#include "tusb.h"

which revealed the implicit dependency via a build break.

@hathach
Copy link
Copy Markdown
Owner

hathach commented Mar 23, 2023

fixing code format by changing the code is not accepted, configure your formatter to skip tinyusb src.

@hathach hathach closed this Mar 23, 2023
@JayToltTech
Copy link
Copy Markdown
Contributor Author

JayToltTech commented Mar 23, 2023

Please take a look at the code change. There's an implicit dependency in tusb_private that I simply made explicit.

Having build order issues in your .h files isn't preferable @hathach. It will likely cause an issue for someone else in the future. As it is, the tusb code is fragile when it doesn't need to be because the dependency isn't explicit so #include order matters when it shouldn't.

In my case, the IDE is simply set to auto format as I go, so adding tusb as a submodule and looking at the code exposes this missing dependency when I open and read tusb.c.

@JayToltTech
Copy link
Copy Markdown
Contributor Author

FYI, saw you opened a (very nice) checklist for static analysis. FYI, your cppcheck run is going to fail with this issue unfixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants