Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
97 changes: 76 additions & 21 deletions src/globals.c
Original file line number Diff line number Diff line change
Expand Up @@ -1381,36 +1381,91 @@ void fatal(char *msg)
/* Reports an error and specifying a position */
void error(char *msg)
{
/* Construct error source diagnostics, enabling precise identification of
* syntax and logic issues within the code.
*/
int offset, start_idx, i = 0;
char diagnostic[512 /* MAX_LINE_LEN * 2 */];
/* Safety check for NULL message */
if (!msg) {
printf("[Error]: Unknown error occurred\n");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the purpose of adding a NULL check here?
AFAICS there is no case where a NULL pointer is passed in, so this doesn't address any real security issue.
Also, I don't see a strong reason to change the API to allow NULL pointers as valid input.

abort();
}

/* Safety check for SOURCE buffer */
if (!SOURCE || !SOURCE->elements || SOURCE->size < 0) {
printf("[Error]: %s\nSource location unavailable (invalid source buffer)\n", msg);
abort();
}
Comment on lines +1391 to +1394
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not correctly handle OOB issue.


/* Handle empty source case */
if (SOURCE->size == 0) {
printf("[Error]: %s\nOccurs at start of file\n", msg);
abort();
}

/* Construct error source diagnostics */
int current_pos = SOURCE->size;
int line_start, line_end;
int i = 0;
char diagnostic[512]; /* MAX_LINE_LEN * 2 */
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally, I would prefer keeping all variable declarations at the top of the function.


/* Ensure current_pos is within bounds */
if (current_pos >= SOURCE->size) {
Copy link

@cubic-dev-ai cubic-dev-ai bot Sep 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bounds check unconditionally shifts the error position one character left, causing off-by-one error in caret/column reporting.

Prompt for AI agents
Address the following comment on src/globals.c at line 1409:

<comment>Bounds check unconditionally shifts the error position one character left, causing off-by-one error in caret/column reporting.</comment>

<file context>
@@ -1381,36 +1381,91 @@ void fatal(char *msg)
+    char diagnostic[512]; /* MAX_LINE_LEN * 2 */
+
+    /* Ensure current_pos is within bounds */
+    if (current_pos &gt;= SOURCE-&gt;size) {
+        current_pos = SOURCE-&gt;size - 1;
+    }
</file context>
Suggested change
if (current_pos >= SOURCE->size) {
if (current_pos > SOURCE->size) {
Fix with Cubic

current_pos = SOURCE->size - 1;
}

for (offset = SOURCE->size; offset >= 0 && SOURCE->elements[offset] != '\n';
offset--)
;
/* Find the start of the current line (scan backwards to find '\n') */
line_start = current_pos;
while (line_start > 0 && SOURCE->elements[line_start - 1] != '\n') {
line_start--;
}

start_idx = offset + 1;
/* Find the end of the current line (scan forwards to find '\n' or end) */
line_end = current_pos;
while (line_end < SOURCE->size && SOURCE->elements[line_end] != '\n') {
line_end++;
}

for (offset = 0;
offset < MAX_SOURCE && SOURCE->elements[start_idx + offset] != '\n';
offset++) {
diagnostic[i++] = SOURCE->elements[start_idx + offset];
/* Copy the current line to diagnostic buffer with bounds checking */
for (int pos = line_start; pos < line_end && i < (int)sizeof(diagnostic) - 50; pos++) {
diagnostic[i++] = SOURCE->elements[pos];
}

/* Add newline after the source line */
if (i < (int)sizeof(diagnostic) - 30) {
diagnostic[i++] = '\n';
}
diagnostic[i++] = '\n';

for (offset = start_idx; offset < SOURCE->size; offset++) {
/* Add spaces to point to the error position */
int error_column = current_pos - line_start;
for (int spaces = 0; spaces < error_column && i < (int)sizeof(diagnostic) - 20; spaces++) {
diagnostic[i++] = ' ';
}

strcpy(diagnostic + i, "^ Error occurs here");
/* Add the error pointer with bounds checking */
const char *pointer_text = "^ Error occurs here";
int pointer_len = strlen(pointer_text);
if (i + pointer_len < (int)sizeof(diagnostic)) {
strcpy(diagnostic + i, pointer_text);
i += pointer_len;
}

/* Null terminate */
if (i < (int)sizeof(diagnostic)) {
diagnostic[i] = '\0';
} else {
diagnostic[sizeof(diagnostic) - 1] = '\0';
}

/* Calculate line number for better error reporting */
int line_number = 1;
for (int pos = 0; pos < current_pos && pos < SOURCE->size; pos++) {
if (SOURCE->elements[pos] == '\n') {
line_number++;
}
}

/* TODO: Implement line/column tracking for precise error location
* reporting. Current implementation only shows source position offset.
*/
printf("[Error]: %s\nOccurs at source location %d.\n%s\n", msg,
SOURCE->size, diagnostic);
/* Output the error with improved formatting */
printf("[Error]: %s\n", msg);
printf("At line %d, column %d (source position %d):\n",
line_number, error_column + 1, current_pos);
printf("%s\n", diagnostic);
Comment on lines +1464 to +1468
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not resolve the goal of TODO, as the line number and column number could only be meaningful when file name persists. With only line number and column number it's quite meaningless.

abort();
}

Expand Down
Loading