Skip to content

tests: drop unused or redundant includes#17717

Closed
vszakats wants to merge 26 commits intocurl:masterfrom
vszakats:t-drop-unused-headers
Closed

tests: drop unused or redundant includes#17717
vszakats wants to merge 26 commits intocurl:masterfrom
vszakats:t-drop-unused-headers

Conversation

@vszakats
Copy link
Member

@vszakats vszakats commented Jun 23, 2025

@testclutch

This comment was marked as off-topic.

@vszakats vszakats force-pushed the t-drop-unused-headers branch from 6ecca03 to b7a8ab8 Compare June 23, 2025 10:01
@vszakats vszakats changed the title tests/libtest: drop unused or redundant includes tests: drop unused or redundant includes Jun 23, 2025
vszakats added 25 commits June 23, 2025 13:48
Also lots of tests were already relying on this.
Some #includes were unguarded. This suggests that this header
is also fine to use without the feature guard and feature check.

(It's a POSIX header)
Some were guarded just by HAVE_PTHREAD_H, instead of its own guard.
@vszakats vszakats force-pushed the t-drop-unused-headers branch from 115cff6 to 73737b0 Compare June 23, 2025 11:48
@vszakats vszakats closed this in 2636828 Jun 23, 2025
@vszakats vszakats deleted the t-drop-unused-headers branch June 23, 2025 11:49
@vszakats
Copy link
Member Author

vszakats commented Jun 23, 2025

I've found it interesting that these headers (all POSIX) were used
unguarded in test code:
fcntl.h, sys/stat.h, unistd.h.

While the builds do the detections, and core code has the feature guards.

There is a fair chance these feature checks are redundant.

edit: unistd.h was dependent on HAVE_PTHREAD_H, and it's missing from some non-Unix platforms.

vszakats added a commit that referenced this pull request Jun 24, 2025
It has been used unconditionally in `src` and `tests` since at least
2011-09-19 via fdecb56. There are
earlier unguarded references in `tests`.

Also de-duplicate to include it just once.

Ref: #17717 (comment)

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

Development

Successfully merging this pull request may close these issues.

2 participants

Comments