tidy-up: drop redundant includes - #22374
Conversation
There was a problem hiding this comment.
Pull request overview
This PR tidies up the codebase by removing redundant system header includes from a unit test and several libcurl source files, relying on existing shared headers (notably curl_setup.h) to provide required platform types and socket definitions consistently.
Changes:
- Remove redundant
#include <sys/types.h>blocks from several lib/vtls sources andlib/file.c. - Remove redundant
#include <sys/types.h>/#include <sys/socket.h>blocks fromtests/unit/unit1961.c.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/unit1961.c | Drops redundant system includes; required platform headers are already pulled via unitcheck.h → first.h → curl_setup.h. |
| lib/vtls/vtls.c | Removes redundant sys/types.h include in favor of curl_setup.h’s common header set. |
| lib/vtls/vtls_scache.c | Removes redundant sys/types.h include; curl_setup.h already provides it when available. |
| lib/vtls/vtls_config.c | Removes redundant sys/types.h include; keeps includes minimal and consistent with other vtls sources. |
| lib/file.c | Removes redundant sys/types.h include; relies on curl_setup.h for platform type definitions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
lib/curl_setup.h:833
- After removing the early
<sys/types.h>include,curl_setup.hshould still include<sys/types.h>(and do so after config headers, but before<sys/stat.h>which can depend on it). Otherwise code that relied oncurl_setup.hto provideoff_t/size_t-related typedefs may break.
#include <time.h>
#include <errno.h>
#include <sys/stat.h>
"`<sys/types.h>` is now included before the configuration headers (`curl_config.h` / `config-*.h`). Some configs set feature macros like `_FILE_OFFSET_BITS` (e.g. `config-win32.h` for MinGW) that must be defined before including system headers like `<sys/types.h>` to get correct typedefs (notably `off_t`). Including `<sys/types.h>` this early can lock in the wrong definitions. This issue also appears on line 830 of the same file."
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
lib/curl_setup.h:510
curl_setup.hnow includes<sys/types.h>unconditionally here, but the file already includes it under#ifdef __APPLE__near the top. This introduces a redundant double-include on Apple and also bypasses the existingHAVE_SYS_TYPES_Hfeature check that other source files still use for portability.
#include <limits.h>
#include <sys/types.h>
| /* Include after setting system macros that may affect type sizes | ||
| (e.g. 'off_t' or 'time_t'), or suppress warnings | ||
| (e.g. '_CRT_SECURE_NO_WARNINGS`), but before including sys/stat.h */ | ||
| #include <sys/types.h> |
There was a problem hiding this comment.
Wrong, it was already included unconditionally via curl/curl.h:
Lines 73 to 74 in 8734b08
The __APPLE__ check is overkill, the redundant include was pre-existing and also benign. (and a tough challenge to avoid is possible at all)
- schannel: drop redundant parentheses. - os400sys: drop redundant includes. Follow-up to ebc5212 #22374 - pytest: replace `()` with `[]` to match rest of tests. - libtests: constify some local pointers. - libtests: drop redundant `(long)` casts. - lib650: use `CURL_CSTRLEN()`. Follow-up to 59dc2bb #22424 Closes #22444
sys/types.handsys/socket.h(non-Win32). They are included viacurl/curl.handcurl_setup.h.This drops
HAVE_SYS_TYPES_Hguards from the codebase. It's safebecause
sys/types.h(POSIX) is already required unconditionally bycurl/curl.h. It remains used in feature checks by both autotools andcmake; to be reviewed in a future step.