Lada1#18
Open
Marcel-git666 wants to merge 48 commits into
Open
Conversation
- Add expansion_size.c with proper buffer size calculation - Fix buffer allocation in init_expansion_state() - Replace fixed formula (input_len * 2) with dynamic calculation - Add t_size_calc structure to avoid norminette violations - Fixes segfault when expanding long environment variables like /home/mmravec/core The previous allocation of strlen(input) * 2 + 1 was insufficient when expanded variables were longer than the original variable names. For example: /home/mmravec/core (7 chars) -> /home/user/long/path (20+ chars)~
…to lada1 # Conflicts: # src/execution_fork.c
This commit introduces more robust error handling and memory management, primarily focusing on scenarios where memory allocation (`malloc`, `strdup`, etc.) can fail. These changes prevent potential segmentation faults and memory leaks under low-memory conditions. Key changes: - **Improved Parser Error Handling:** - The `fill_argument_arrays` function now returns a status code to signal allocation failures from `ft_strdup`. - The `free_command_node` function was refactored to safely deallocate partially initialized command nodes. This prevents crashes when cleaning up after a failed allocation. - The `parse_command` function now uses the improved `free_ast` for cleaner and more reliable cleanup, making the code more readable and maintainable. - **Safer File Descriptor Allocation:** - Added a `NULL` check in `execute_command` for the return value of `set_fd()`. This prevents a segmentation fault if `malloc` fails during the creation of the `t_fds` structure.
This commit improves the error handling within the assignment parsing logic. - The `split_assignment_parts` function now returns a status code to signal failures from `ft_strndup` or `ft_strdup`. It also ensures that partially allocated memory is cleaned up internally. - The `parse_assignment` function was simplified to check this new status code and now calls `free_ast` for unified and robust cleanup, removing the need for manual checks and the `cleanup_assignment_error` helper function.
This commit adds a `NULL` check for the return value of `ft_strdup` within the `create_redirection` function. If the allocation for `file_or_delimiter` fails, the function now correctly frees the `t_redirection` struct and returns `NULL`, preventing a partially initialized struct from being returned and causing issues later in the parsing process.
This commit hardens the `create_token` function to prevent returning a partially initialized token. It now checks the return value of `ft_strdup` for the token's value. If this allocation fails, the function frees the `t_token` struct itself and returns `NULL`. This makes the function atomic: it either returns a fully valid token or `NULL`, ensuring callers do not have to handle tokens with `NULL` values, thus preventing segmentation faults.
This commit addresses a memory leak that occurred when `execve` failed within a forked child process. Previously, only the `args` array was freed, leaving memory allocated for the command `path` and potentially a dynamic `envp` array leaked. - The `cleanup_child_process` function has been extended to accept and deallocate the `path` and `envp` pointers. - The `fork_it` function now passes all necessary pointers to the cleanup function in its error-handling path. This ensures that child processes that fail to execute clean up all their allocated resources before exiting, making the shell more robust against memory leaks.
# Conflicts: # src/execution_fork.c
# Conflicts: # Makefile # include/builtins.h # include/expansion.h # src/builtin_cd.c # src/builtin_exit.c # src/execution_fork.c # src/execution_search.c # src/expansion.c # src/history.c # src/pipes_execution.c # src/signals.c
…to lada1 # Conflicts: # src/execution_fork.c
…to lada1 # Conflicts: # src/execution_fork.c
This major refactor overhauls the parsing and execution logic to correctly handle commands with interleaved arguments and redirections, bringing behavior in line with shells like bash.
Previously, the parser would stop collecting arguments for a command as soon as it encountered a redirection token. This led to incorrect behavior for commands such as `echo "arg1" > file "arg2"`, where `"arg2"` would be ignored.
Key changes:
- **Iterative Parsing:** The parser (`parser.c`) now iterates through tokens until a pipe (`|`) or the end of the line, building a single command node (`NODE_COMMAND`).
- **Redirection List:** Redirections are now stored as a linked list (`t_redirection`) within the command node, rather than being separate nodes in the AST. This simplifies the tree structure.
- **Dynamic Argument Handling:** Removed the initial `count_arguments` logic in favor of a more flexible approach that adds arguments one by one without using the forbidden `realloc` function.
- **Executor Synchronization:** The execution logic (`execution.c`) has been updated to understand the new AST structure, correctly processing the linked list of redirections before executing the command.
- **Bug Fixes:** This refactoring also addressed several related issues discovered during the process:
- Corrected signal handling for `heredoc` (`Ctrl+C`) to prevent the shell from hanging.
- Fixed memory management issues (double frees, leaks) in the `cd` builtin.
- Replaced the forbidden `mkstemp` function with a custom temporary file generator (`new_tempfile`) in `heredoc`.
- Removed a significant amount of dead or obsolete code from the old parser implementation, resulting in a cleaner and more maintainable codebase.
The shell is now significantly more robust and correctly handles a wider range of complex command-line inputs as required by the subject.
…into lada1 # Conflicts: # include/redirection.h # src/builtin_exit.c # src/execution_search.c # src/export.c # src/parser.c # src/parser_utils.c # src/redirection.c # src/signals.c
This commit introduces a major refactor of the core parsing and execution logic to correctly handle complex commands, improve POSIX compliance, and fix critical bugs related to pipes, redirections, and signal handling.
### Key Changes:
1. **Robust Parser for Complex Commands:**
* The parser no longer stops collecting arguments upon encountering a redirection. It now correctly handles interleaved arguments and redirections (e.g., `echo arg1 > file arg2`).
* Redirections are now parsed into a linked list within the command node, simplifying the AST and correctly handling multiple redirections (e.g., `> file1 > file2`).
* Enhanced syntax error detection for invalid pipe usage (`| wc`, `ls |`), now returning the correct exit code (2) and displaying an error message.
2. **Correct Pipe Exit Status (`$?`):**
* The exit code of a pipeline is now correctly determined *only* by the status of the last command in the sequence, bringing the behavior in line with `bash`.
3. **Strict `export` Validation:**
* The `export` builtin now uses a robust validation function (`is_valid_var_name`) to reject invalid identifiers (e.g., `VAR-NAME`, `9VAR`) and sets the appropriate error code.
4. **Stable `heredoc` and Signal Handling (`Ctrl+C`):**
* Fixed a critical bug where the shell would hang after pressing `Ctrl+C` during a `heredoc` prompt.
* This was resolved by correctly saving and restoring terminal settings (`tcgetattr`/`tcsetattr`) around the `fork` call, ensuring the parent process always reclaims a sane terminal.
5. **Subject Compliance and Bug Fixes:**
* Replaced the forbidden `mkstemp` function with a compliant custom temporary file generator (`new_tempfile`).
* Eliminated numerous memory management issues, including double frees and memory leaks, particularly within the `cd` builtin and command execution paths.
* Refactored the codebase to remove a significant amount of dead code from the old parser, improving clarity and maintainability.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.