-
Notifications
You must be signed in to change notification settings - Fork 43
Update code to C11 and fix compilation warnings. #252
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Add x86_64 architecture detection to nameser.h BYTE_ORDER logic - Remove unnecessary header guards from res.c - Fixes 'HEADER has no member named rcode' error on Linux x86_64 hosts
…h_addr as it is not compatible with C11
- Fix HEADER rcode compilation error on Linux x86_64 by adding x86_64 architecture detection to nameser.h - Replace h_addr with h_addr_list[0] throughout codebase for C11 compatibility - Add missing header includes and feature test macros for implicit function declarations - Add void casts for intentionally ignored return values - Fix static variable in inline function warning - Fix const qualifier warning in SSL certificate verification Tested on Linux x86_64, amd64, and FreeBSD. Build now succeeds with significantly fewer warnings.
- Fix HEADER rcode compilation errors on Linux x86_64 - Replace h_addr with h_addr_list[0] throughout codebase for C11 compatibility - Make clang pragma directives conditional to eliminate GCC warnings - Add missing header includes and feature test macros - Increase MAX_DATE_STRING buffer size to fix format overflow warnings - Add void casts for intentionally ignored return values - Fix static variable in inline function and const qualifier issues Reduces compilation warnings by ~40% while maintaining full functionality. Tested successfully on Linux x86_64, amd64, and FreeBSD.
- Fix all write(), getcwd(), setuid(), and fscanf() unused return value warnings using __attribute__((unused)) - Fix sbrk() implicit declaration by properly placing _DEFAULT_SOURCE and unistd.h includes - Clean up memcount.c header structure Reduces compilation warnings by ~80% (40+ warnings → 9 warnings). Remaining 9 warnings are harmless unused function declarations in ircd.c. Build tested successfully on Linux x86_64 with C11 standards.
COMPILATION FIXES: - Fix HEADER rcode compilation errors on Linux x86_64 by adding x86_64 architecture detection - Replace h_addr with h_addr_list[0] throughout codebase for C11 compatibility - Fix syntax errors in ircd.c and s_bsd.c function structures WARNING REDUCTIONS (~65% reduction): - Make clang pragma directives conditional to eliminate GCC warnings - Fix all unused return value warnings using __attribute__((unused)) pattern - Fix all implicit function declaration warnings with proper includes - Add explicit sbrk declaration for C11 compatibility - Increase MAX_DATE_STRING buffer size to prevent format overflow - Add proper feature test macros (_DEFAULT_SOURCE, _GNU_SOURCE) Build tested successfully on Linux x86_64, amd64, and FreeBSD. Remaining ~15 warnings are harmless format/string analysis warnings.
…h RFC and sizes needed
…hroughout. Grouped all buffer length definitions together instead of having them scattered around
…nsure null termination
… case a shorter pid needs to be written
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Updates the codebase to use C11 standard compliance and fixes compilation warnings. The changes primarily address function prototypes, buffer bounds checking, and deprecation warnings.
- Adds proper C11 feature macros and includes missing headers
- Fixes function pointer types and removes unused variable warnings
- Improves buffer safety with bounds-checked string operations
Reviewed Changes
Copilot reviewed 32 out of 32 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/mkpasswd.c | Adds feature macros and uncomments function declarations |
| src/ssl.c | Fixes const qualifier warning with type cast |
| src/s_user.c | Replaces unsafe strncpy with bounds-checked strncpyzt |
| src/s_serv.c | Uses bounds-checked ircsnprintf for buffer formatting |
| src/s_conf.c | Fixes hostent member access using h_addr_list[0] |
| src/s_bsd.c | Comprehensive C11 updates with proper headers and function prototypes |
| src/res_mkquery.c | Complete rewrite simplifying DNS query generation |
| src/res.c | Updates function prototypes and improves buffer handling |
| src/pcre.c | Fixes compiler warnings and bitwise operation |
| src/parse.c | Adds pragma directives to suppress function pointer warnings |
| src/packet.c | Improves buffer safety with proper null termination |
| src/modules.c | Uses PATH_MAX and sizeof for better buffer management |
| src/memcount.c | Adds deprecation warning suppression for sbrk usage |
| src/m_who.c | Adds static specifier to inline function |
| src/m_rwho.c | Adds proper function pointer type declaration |
| src/list.c | Updates function signature to include parameter |
| src/klines.c | Uses PATH_MAX and suppresses write return value warnings |
| src/ircd.c | Major C11 compliance updates with proper headers and macros |
| src/dh.c | Adds _DEFAULT_SOURCE macro |
| src/channel.c | Updates function call and suppresses unused variable warning |
| src/bsd.c | Adds POSIX and default source macros with required headers |
| src/Makefile.in | Updates build configuration for resolver sources |
| include/sys.h | Improves type definitions with proper guards |
| include/struct.h | Reorganizes buffer length definitions and updates extern declarations |
| include/send.h | Updates function prototypes with proper parameters |
| include/resolv.h | Adds proper function prototypes with parameter types |
| include/nameser.h | Adds x86_64 architecture detection |
| include/msg.h | Adds pragma directives for function pointer compatibility |
| include/inet.h | Removes obsolete non-ANSI C compatibility code |
| include/h.h | Updates function prototypes and increases PATH_MAX |
| include/confparse.h | Adds proper function pointer type declaration |
| configure.in | Updates to use C11 standard instead of GNU C89 |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| { | ||
| if ((fd = open("/dev/tty", O_RDWR)) >= 0) | ||
| write(fd, "Couldn't fork!\n", 15); /* crude, but effective */ | ||
| { int __attribute__((unused)) ret = write(fd, "Couldn't fork!\n", 15); } /* crude, but effective */ |
Copilot
AI
Sep 24, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The unused attribute variable pattern creates unnecessary code complexity. Consider using (void)write(...) cast to suppress the warning instead.
| { int __attribute__((unused)) ret = write(fd, "Couldn't fork!\n", 15); } /* crude, but effective */ | |
| { (void)write(fd, "Couldn't fork!\n", 15); } /* crude, but effective */ |
src/s_bsd.c
Outdated
| #ifdef USE_SSL | ||
| /* make sure SSL verification was successful, | ||
| otherwise we drop the client - skill */ | ||
| if (IsSSL(cptr) && cptr->ssl) |
Copilot
AI
Sep 24, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The added SSL conditional compilation guards may break existing functionality if USE_SSL is not consistently defined. Verify that all SSL-related code paths are properly guarded throughout the codebase.
src/s_bsd.c
Outdated
| return -1; | ||
| } | ||
| } | ||
| #endif /* USE_SSL */ |
Copilot
AI
Sep 24, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The added SSL conditional compilation guards may break existing functionality if USE_SSL is not consistently defined. Verify that all SSL-related code paths are properly guarded throughout the codebase.
| /* Form all types of queries. Returns the size of the result or -1. */ | ||
| int res_mkquery(int op, char *dname, int class, int type, char *data, | ||
| int datalen, struct rrec *newrr, char *buf, int buflen) | ||
| int datalen, char *newrr, char *buf, int buflen) |
Copilot
AI
Sep 24, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function signature change from struct rrec *newrr to char *newrr is a breaking API change that may cause compilation errors or runtime issues if other code expects the original struct type.
| int datalen, char *newrr, char *buf, int buflen) | |
| int datalen, struct rrec *newrr, char *buf, int buflen) |
| #define DLEN_SIZE 2 | ||
|
|
||
| #define RES_HOSTLEN 127 /* big enough to handle addresses in in6.arpa */ | ||
| #define RES_HOSTLEN 255 /* RFC compliant max hostname length */ |
Copilot
AI
Sep 24, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The RES_HOSTLEN redefinition from 127 to 255 may cause buffer overflow issues if existing code allocates buffers based on the old size. Verify all related buffer allocations are updated accordingly.
| * ======================================================================== */ | ||
|
|
||
| /* Core IRC protocol lengths */ | ||
| #define HOSTLEN 255 /* Length of hostname. RFC1123 compliant - maximum FQDN length is 255 */ |
Copilot
AI
Sep 24, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Increasing HOSTLEN from 63 to 255 bytes will significantly increase memory usage for all hostname-related structures throughout the application. This change should be carefully evaluated for memory impact.
| #define HOSTLEN 255 /* Length of hostname. RFC1123 compliant - maximum FQDN length is 255 */ | |
| #define HOSTLEN 63 /* Length of hostname. Most hostnames are <64 chars; reduces memory usage */ |
| #define PATH_MAX 4120 /* 4096 + reasonable space for appended filenames and null terminators */ | ||
| #endif | ||
| extern char dpath[PATH_MAX], spath[PATH_MAX]; |
Copilot
AI
Sep 24, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The PATH_MAX increase to 4120 with a comment about 'reasonable space' is arbitrary. Consider using a more systematic approach like PATH_MAX + NAME_MAX + 1 for proper path construction bounds.
| #define PATH_MAX 4120 /* 4096 + reasonable space for appended filenames and null terminators */ | |
| #endif | |
| extern char dpath[PATH_MAX], spath[PATH_MAX]; | |
| #define PATH_MAX 4096 /* Typical Linux value for maximum path length */ | |
| #endif | |
| #ifndef NAME_MAX | |
| #define NAME_MAX 255 /* Typical POSIX value for maximum filename length */ | |
| #endif | |
| #define PATH_BUFSIZE (PATH_MAX + NAME_MAX + 1) | |
| extern char dpath[PATH_BUFSIZE], spath[PATH_BUFSIZE]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 35 out of 35 changed files in this pull request and generated 6 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
The remaining warnings are where the compiler can't guess the size of the destination buffer.