Skip to content

Conversation

EricccTaiwan
Copy link
Contributor

When the console input exceeds the error limit (five attempts), it displays: "Error limit exceeded. Stopping command execution." However, it does not exit the command prompt. Additionally, attempting to quit using the "quit" command fails, leaving Ctrl+C as the only way to terminate the program, which may lead to memory leaks.

This commit fixes the issue by declaring do_quit before record_error and calling it upon reaching the error limit to prevent memory leaks.

Alternative solutions considered:

  1. Rearranging function definitions to place do_quit before record_error, which eliminates the need for declaration but requires restructuring, a change that warrants further discussion.
  2. Setting quit_flag after exceeding the limit and enforcing "quit" as the only allowed input, preserving the existing function order.

This approach balances maintainability and immediate resolution, ensuring the console exits properly once the input limit is exceeded.

Change-Id: I52d53f69156bff2ab793afba65b46c67c125671f

console.c Outdated
report(
1,
"Error limit exceeded. Stopping command execution, and quitting");
do_quit(0, NULL);
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't do this. The functions starting with do_ are only used by the command interpreter during command dispatching. You can introduce another helper function for both do_quit and record_error.

Copy link
Contributor Author

@EricccTaiwan EricccTaiwan Mar 9, 2025

Choose a reason for hiding this comment

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

I have introduced a helper function, force_quit, in the latest commit to handle both do_quit and record_error. This ensures that functions starting with do_ remain strictly for command dispatching while reducing code duplication and improving maintainability.

Copy link
Contributor

@jserv jserv left a comment

Choose a reason for hiding this comment

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

Squash commits.

- Fixed issue where exceeding the error limit (five attempts) did
  not exit the console, potentially leading to memory leaks.
- Introduced "force_quit" as a helper function to centralize quit
  handling.
- Ensured both "do_quit" and "record_error" call "force_quit",
  enforcing a consistent function structure.
- Reduced code duplication and improved command dispatching logic.

Co-authored-by: charliechiou <[email protected]>
Change-Id: I2f624a16cc4eb3517bf05a65eac105908300bfd9
@jserv jserv merged commit 4bb347c into sysprog21:master Mar 9, 2025
1 of 2 checks passed
@jserv
Copy link
Contributor

jserv commented Mar 9, 2025

Thank @EricccTaiwan for contributing!

@EricccTaiwan EricccTaiwan deleted the fix-record_error branch March 11, 2025 03:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants