Conversation
…ng object and removed all warnings
- Updated comment formatting for consistency and clarity. - Adjusted spacing and indentation for better readability. - Ensured that all comments are properly aligned and formatted. - Minor code style improvements for better maintainability.
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the AFC String API by adding JavaScript-like string manipulation methods, making the library more familiar to developers with JavaScript experience. The changes also include important security fixes for buffer handling and documentation improvements.
Key Changes:
- Added 13 new JavaScript-style string methods (charAt, startsWith, endsWith, repeat, replace, replaceAll, padStart, padEnd, slice, indexOf, lastIndexOf, trimStart, trimEnd, fromCharCode)
- Fixed buffer overflow vulnerabilities in fileops.c and dirmaster.c by using bounded string operations
- Added fall-through comments to eliminate compiler warnings in switch statements
- Improved code formatting and consistency in PCRE library files
Reviewed Changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/string.h | Added function prototypes and macros for new JS-like string methods (includes, char_code_at) |
| src/string.c | Implemented 13 new string manipulation functions with comprehensive documentation; added fall-through comments; replaced abs() with labs() for long integers; improved conditional expression readability |
| src/tree.c | Added wrapper function to properly handle function pointer types in tree traversal |
| src/fileops.c | Fixed buffer overflow by using snprintf instead of sprintf and correcting buffer size constant |
| src/dirmaster.h | Increased buffer sizes for user/group names (10→32) and file size strings (10→20) to prevent truncation |
| src/dirmaster.c | Added proper null-termination for strncpy operations; updated snprintf calls with correct buffer sizes; improved format string for decimal display |
| src/base.c | Fixed typo in documentation comment; added GCC diagnostic pragmas to suppress false-positive warning |
| src/pcre/study.c | Code formatting improvements (indentation, spacing) and comment style updates |
| src/pcre/pcregrep.c | Code formatting improvements and replaced sprintf with snprintf for safer buffer handling |
| internal/string.md | Added comprehensive JavaScript String API reference documentation (new file) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| NAME: afc_string_from_char_code ( code ) - Returns a string created from the specified UTF-16 code unit | ||
|
|
||
| SYNOPSIS: char * afc_string_from_char_code ( int code ) | ||
|
|
||
| DESCRIPTION: Returns a string created from the specified char code. | ||
|
|
||
| INPUT: - code - The char code. | ||
|
|
||
| RESULT: - A new AFC string containing the character. |
There was a problem hiding this comment.
The comment says "UTF-16 code unit" but the function signature accepts int code and simply casts it to char. This function creates a single-byte character from a code, not a UTF-16 code unit. The documentation comment should be corrected to accurately reflect that this creates a string from a single byte value (0-255), not a UTF-16 code unit.
| NAME: afc_string_from_char_code ( code ) - Returns a string created from the specified UTF-16 code unit | |
| SYNOPSIS: char * afc_string_from_char_code ( int code ) | |
| DESCRIPTION: Returns a string created from the specified char code. | |
| INPUT: - code - The char code. | |
| RESULT: - A new AFC string containing the character. | |
| NAME: afc_string_from_char_code ( code ) - Returns a string created from the specified single byte value (0-255) | |
| SYNOPSIS: char * afc_string_from_char_code ( int code ) | |
| DESCRIPTION: Returns a string created from the specified single byte value (0-255). | |
| INPUT: - code - The byte value (0-255) to use as the character. | |
| RESULT: - A new AFC string containing the single byte character. |
| char *afc_string_from_char_code(int code) | ||
| { | ||
| char *s = afc_string_new(1); | ||
| if (s) | ||
| { | ||
| s[0] = (char)code; | ||
| s[1] = '\0'; | ||
| afc_string_reset_len(s); | ||
| } | ||
| return s; | ||
| } |
There was a problem hiding this comment.
Memory leak potential: If afc_string_new(1) fails and returns NULL, the function returns NULL without any error handling. However, if it succeeds, the caller must call afc_string_delete() to free the memory. The documentation should explicitly state that the returned string must be freed by the caller, or consider adding a note in the function comment about memory management.
| if (fromIndex < 0) | ||
| return -1; // JS behavior: if fromIndex < 0, treated as 0, but since we search backwards from 0, only match at 0 is possible if searchLen is 0, else -1. Actually JS treats negative as 0. |
There was a problem hiding this comment.
The comment on lines 1918-1919 is confusing and appears to contain conflicting information about JavaScript behavior. The comment states "JS behavior: if fromIndex < 0, treated as 0" but then immediately returns -1, which contradicts this statement. According to JavaScript spec, when fromIndex is negative in lastIndexOf, it's treated as length + fromIndex (clamped to 0). The current implementation simply returns -1 for negative indices, which doesn't match JavaScript behavior. Consider implementing: if (fromIndex < 0) fromIndex = 0; to match the described behavior.
| if (fromIndex < 0) | |
| return -1; // JS behavior: if fromIndex < 0, treated as 0, but since we search backwards from 0, only match at 0 is possible if searchLen is 0, else -1. Actually JS treats negative as 0. | |
| // JS behavior: if fromIndex < 0, treated as len + fromIndex, clamped to 0 | |
| if (fromIndex < 0) { | |
| fromIndex = len + fromIndex; | |
| if (fromIndex < 0) | |
| fromIndex = 0; | |
| } |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Adding some methods to make AFC String much more similar to javascript String object