init/dhcp: only overwrite resolv.conf with DNS

[slp]

Only overwrite resolv.conf when the DHCP server has provide us some nameservers. This allows users to keep the original resolv.conf in case it was populated beforehand.

[slp]

/gemini review

[gemini-code-assist]

Code Review

This pull request defers the opening of /etc/resolv.conf to only when DHCP Option 6 (DNS) is present in the response. The reviewer identified a potential file descriptor leak if multiple Option 6 fields are processed, as well as the loss of error reporting when fopen fails. A code suggestion was provided to conditionally open the file and restore the error logging.

[gemini-code-assist]

If the DHCP response contains multiple Option 6 (DNS) fields, calling fopen repeatedly without checking if resolv is already open will leak the previous file descriptor. Additionally, the error reporting (perror) when fopen fails has been lost in this change. Checking if resolv is already open and restoring the error reporting solves both issues.

            if (!resolv) {
                resolv = fopen("/etc/resolv.conf", "w");
                if (!resolv) {
                    perror("Failed to open /etc/resolv.conf");
                }
            }
            if (resolv) {

[slp]

/gemini review

[gemini-code-assist]

Code Review

This pull request defers the opening of /etc/resolv.conf in init/dhcp.c to be lazy, only attempting to open the file when a Domain Name Server option (Option 6) is actually processed. The review feedback points out that if this lazy fopen call fails, the error is now silently ignored, and recommends adding error handling that aligns with the file's existing logging patterns.

[gemini-code-assist]

When fopen fails to open /etc/resolv.conf, the error is silently ignored. We should report this error, but we must prioritize consistency with the existing error reporting patterns in this file (e.g., using stdout vs stderr or a specific logging function) to maintain a uniform codebase, rather than automatically using perror.

            if (!resolv && !tried_opening_resolv) {
                tried_opening_resolv = true;
                resolv = fopen("/etc/resolv.conf", "w");
                if (!resolv) {
                    // Use the consistent error reporting pattern of this file
                }
            }

References

1. Prioritize consistency with existing error reporting patterns in a file (e.g., using stdout vs stderr) over standard external practices to maintain a uniform codebase.

[slp]

/gemini review

[gemini-code-assist]

Code Review

This pull request defers the opening of /etc/resolv.conf in handle_dhcp_ack so that it is only opened lazily when a DNS option (Option 6) is encountered in the DHCP response. The review feedback highlights two key areas for improvement: first, ensuring that fclose is guarded against NULL pointers and that early returns do not leak the file descriptor; second, simplifying resource management by localizing the file operations within the DNS option block and replacing the magic number 6 with a named constant.

[gemini-code-assist]

Since resolv is now initialized to NULL and only opened conditionally when a DNS option is present, any subsequent call to fclose(resolv) at the end of handle_dhcp_ack must be guarded with a NULL check (i.e., if (resolv) fclose(resolv);) to prevent a segmentation fault when no DNS options are provided in the DHCP response.

Additionally, if handle_dhcp_ack contains any early return paths after the options loop, resolv must be closed before returning to prevent a file descriptor leak.

[gemini-code-assist]

To simplify resource management and eliminate function-level state, consider localizing the file opening and closing directly within the DNS option block. Additionally, avoid using the magic number 6 directly; define it as a named constant (e.g., DHCP_OPTION_DNS) for improved readability and maintainability. Since all DNS servers are typically provided within a single option block, you can open /etc/resolv.conf, write the nameservers, and close it immediately within this block. This avoids the need for the function-level resolv and tried_opening_resolv variables, and completely eliminates the risk of leaks on early returns or crashes from fclose(NULL).

References

1. When using magic numbers for DHCP options, define them as named constants for improved readability and maintainability.