Skip to content

Conversation

@eriksjolund
Copy link
Contributor

Not important but if we aim for no leaks and correct errno then here is another one.

Signed-off-by: Erik Sjölund <erik.sjolund@gmail.com>
@gemini-code-assist
Copy link

Summary of Changes

Hello @eriksjolund, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request enhances the robustness of container creation by improving error handling in the libcrun_container_create function. It ensures that when a configuration file copy operation fails, the system accurately captures the specific error code and properly releases associated error resources, leading to more precise error reporting and preventing resource leaks.

Highlights

  • Error Code Extraction: The pull request refines error handling within the libcrun_container_create function by ensuring the correct error code is extracted from the err object using crun_error_get_errno.
  • Resource Release: The crun_error_release function is now explicitly called to free the error object, preventing potential resource leaks when libcrun_copy_config_file fails.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request correctly identifies and fixes an issue with error handling where the global errno was used instead of the one from the error object, and also fixes a memory leak by releasing the error object.

My review includes one critical suggestion to further improve the fix. The current change uses libcrun_fail_with_error, which terminates the process. Library functions like libcrun_container_create should return errors to the caller instead of exiting. The proposed change replaces the call to libcrun_fail_with_error with a return statement using crun_make_error, which is consistent with other error handling paths in the same function.

{
int errcode = crun_error_get_errno (err);
crun_error_release (err);
libcrun_fail_with_error (errcode, "copy config file");

Choose a reason for hiding this comment

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

critical

Thank you for fixing the error handling here. This correctly uses the errno from the err object and prevents a memory leak. However, libcrun_container_create is a library function that should return an error to its caller, not terminate the process with exit(). The libcrun_fail_with_error function is noreturn.

A few lines below, for the error from libcrun_container_run_internal, the code correctly returns an error. We should follow the same pattern here and use crun_make_error to create and return an error.

      return crun_make_error (err, errcode, "copy config file");

@packit-as-a-service
Copy link

TMT tests failed. @containers/packit-build please check.

Copy link
Member

@giuseppe giuseppe left a comment

Choose a reason for hiding this comment

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

LGTM

@giuseppe giuseppe merged commit 2e5803b into containers:main Feb 1, 2026
41 of 43 checks passed
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