Skip to content

utils: refactor set_home_env#2011

Open
eriksjolund wants to merge 1 commit intocontainers:mainfrom
eriksjolund:refactor-set_home_env
Open

utils: refactor set_home_env#2011
eriksjolund wants to merge 1 commit intocontainers:mainfrom
eriksjolund:refactor-set_home_env

Conversation

@eriksjolund
Copy link
Contributor

@eriksjolund eriksjolund commented Feb 12, 2026

Use the return value from fgetpwent_r as error value. Reference:
https://man7.org/linux/man-pages/man3/fgetpwent_r.3.html#RETURN_VALUE

Closes: #2010

@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 refines the error handling mechanism within the set_home_env utility function. By directly leveraging the return value of fgetpwent_r for error detection and ensuring errno is appropriately set, the changes improve the robustness and clarity of error propagation, particularly when a user entry is not found.

Highlights

  • Error Handling Refactor: The set_home_env function's error handling has been refactored to align with the specified return values of fgetpwent_r, directly using its return for error checks.
  • errno Propagation: The errno variable is now explicitly set to the return value of fgetpwent_r when an error occurs (excluding ERANGE), ensuring proper error propagation to callers.
  • Code Simplification: Removed an unnecessary ret variable and simplified the final error return statement in set_home_env.

🧠 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.

Changelog
  • src/libcrun/utils.c
    • Removed the ret integer variable as its functionality was integrated directly into the fgetpwent_r call.
    • Modified the error checking for fgetpwent_r to use its direct return value instead of relying on errno for all cases.
    • Ensured that if fgetpwent_r returns an error other than ERANGE, its return value is assigned to errno before jumping to the error handler.
    • Simplified the final return statement in the error block to always return -errno.
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

The changes refactor the set_home_env function to use the return value of fgetpwent_r as the error code, which simplifies the logic and follows a common glibc pattern. The refactoring is well-done. However, I've identified a potential high-severity issue where an infinite loop could occur if fgetpwent_r returns success but with a NULL result pointer, which should be addressed for robustness.

Use the return value from fgetpwent_r as error value.
Reference:
https://man7.org/linux/man-pages/man3/fgetpwent_r.3.html#RETURN_VALUE

Closes: containers#2010

Signed-off-by: Erik Sjölund <erik.sjolund@gmail.com>
@eriksjolund eriksjolund force-pushed the refactor-set_home_env branch from bb087a0 to 4aa4a0f Compare February 12, 2026 13:47
@packit-as-a-service
Copy link

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

Copy link
Collaborator

@kolyshkin kolyshkin left a comment

Choose a reason for hiding this comment

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

I think we can drop error: label and the goto, and do an immediate return right away instead. Also we can return -1 as the callers are not interested in the particular error.

IOW:

  if (errno != ERANGE)
    return -1

Also, callers check for errno != ENOTSUP and I think it should be removed (not needed since commit 6ed7bab).

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.

set_home_env() has an unnecessary ternary operator

2 participants