-
Notifications
You must be signed in to change notification settings - Fork 10
Add datemotd module for date-based MOTD messages #136
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
base: unreal6
Are you sure you want to change the base?
Conversation
This module displays a specific message in the MOTD based on a defined date.
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
This PR introduces a new UnrealIRCd module that displays date-specific messages in the MOTD (Message of the Day). The module supports both specific dates (YYYY-MM-DD format) and recurring weekday-based messages.
Key Changes:
- Implements date-based MOTD functionality with support for specific dates and weekday names
- Adds configuration parsing for datemotd blocks with name, file, and date fields
- Includes date validation and matching logic for both formats
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| struct datemotd_item *next = p->next; | ||
| safe_free(p->name); | ||
| safe_free(p->file); | ||
| safe_free(p->date); // NEW |
Copilot
AI
Nov 11, 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 comment "NEW" is a development artifact that should be removed before finalizing the code. This type of comment is useful during development but serves no purpose in the final codebase.
| safe_free(p->date); // NEW | |
| safe_free(p->date); |
| sendnumeric(client, RPL_MOTD, line); | ||
| } |
Copilot
AI
Nov 11, 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 file reading logic doesn't handle lines that don't end with a newline character (e.g., the last line of a file). If the last line doesn't have a trailing newline, line[len-1] won't be '\n', and the line will be sent with its content intact but won't be processed through the newline removal logic. This could lead to inconsistent behavior. Consider also sending lines that don't end with newlines.
| sendnumeric(client, RPL_MOTD, line); | |
| } | |
| } | |
| sendnumeric(client, RPL_MOTD, line); |
| FILE *fp = fopen(file, "r"); | ||
| if (!fp) | ||
| return; |
Copilot
AI
Nov 11, 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.
There's no error logging when the file cannot be opened. Users won't know why their configured datemotd isn't being displayed. Consider logging a warning or error message to help with troubleshooting.
| const char *days_short[] = { "Sun", "Mon", "Tue", "Wed", "Thu", "Fri", "Sat" }; | ||
| const char *days_long[] = { "Sunday", "Monday", "Tuesday", "Wednesday", "Thursday", "Friday", "Saturday" }; | ||
|
|
||
| char lower_input[16]; | ||
| int i; | ||
| for (i = 0; i < sizeof(lower_input)-1 && datestr[i]; i++) | ||
| lower_input[i] = tolower((unsigned char)datestr[i]); | ||
| lower_input[i] = '\0'; | ||
|
|
||
| for (i = 0; i < 7; i++) { | ||
| char tmp[16]; | ||
| snprintf(tmp, sizeof(tmp), "%s", days_short[i]); | ||
| for (int j = 0; tmp[j]; j++) tmp[j] = tolower((unsigned char)tmp[j]); | ||
| if (!strcmp(lower_input, tmp)) return 1; | ||
|
|
||
| snprintf(tmp, sizeof(tmp), "%s", days_long[i]); | ||
| for (int j = 0; tmp[j]; j++) tmp[j] = tolower((unsigned char)tmp[j]); | ||
| if (!strcmp(lower_input, tmp)) return 1; | ||
| } |
Copilot
AI
Nov 11, 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 weekday name arrays and lowercase conversion logic are duplicated in three places: is_valid_date_string (lines 72-90), is_today (lines 112-128), and the arrays appear again in is_today. Consider extracting this into a helper function or using constants to avoid code duplication and reduce maintenance burden.
| return 0; | ||
|
|
||
| time_t now = time(NULL); | ||
| struct tm *tm_now = localtime(&now); |
Copilot
AI
Nov 11, 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.
localtime() returns a pointer to static internal storage that may be overwritten by subsequent calls. While this is likely safe in this specific context, it's not thread-safe. Consider using localtime_r() for thread safety if the UnrealIRCd framework supports multi-threaded operations.
| } | ||
|
|
||
|
|
||
| // NEW: hook to send MOTD only on the matching date |
Copilot
AI
Nov 11, 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 comment "NEW: hook to send MOTD only on the matching date" is a development artifact that should be removed. This type of comment is useful during development but serves no purpose in the final codebase.
| // NEW: hook to send MOTD only on the matching date |
| char tmp[16]; | ||
| snprintf(tmp, sizeof(tmp), "%s", days_short[i]); | ||
| for (int j = 0; tmp[j]; j++) tmp[j] = tolower((unsigned char)tmp[j]); |
Copilot
AI
Nov 11, 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.
Variable j is declared in the for loop initialization, which requires C99 or later. While this is likely supported, ensure consistency with the rest of the codebase's C standard usage. The same pattern appears on line 88.
| if (isdigit(datestr[0])) { | ||
| int y, m, d; | ||
| if (sscanf(datestr, "%d-%d-%d", &y, &m, &d) != 3) | ||
| return 0; | ||
| if (y < 1970 || m < 1 || m > 12 || d < 1 || d > 31) | ||
| return 0; |
Copilot
AI
Nov 11, 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 date validation doesn't account for valid days per month (e.g., February 31st would pass validation). Consider adding proper date validation that checks the actual number of days in each month, including leap years.
| const char *days_long[] = { "Sunday", "Monday", "Tuesday", "Wednesday", "Thursday", "Friday", "Saturday" }; | ||
|
|
||
| char lower_input[16]; | ||
| int i; |
Copilot
AI
Nov 11, 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 buffer size check i < sizeof(lower_input)-1 should use strlen(datestr) or ensure the input length doesn't exceed the buffer. If datestr is longer than 15 characters, it will be silently truncated, which could lead to unexpected behavior for invalid inputs.
| int i; | |
| int i; | |
| // Reject input longer than buffer size (15 chars) | |
| if (strlen(datestr) > sizeof(lower_input) - 1) | |
| return 0; |
| struct datemotd_item { | ||
| char *name; | ||
| char *file; | ||
| char *date; // NEW: date in format YYYY-MM-DD |
Copilot
AI
Nov 11, 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 comment "NEW: date in format YYYY-MM-DD" is incomplete and misleading. The date field actually supports both YYYY-MM-DD format and weekday names (e.g., "Monday", "Mon"). Update the comment to reflect all supported formats.
| char *date; // NEW: date in format YYYY-MM-DD | |
| char *date; // Date in format YYYY-MM-DD or weekday name (e.g., "Monday", "Mon") |
This module displays a specific message in the MOTD based on a defined date.