Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

getifaddrs support returning multiple IPv6 addresses #14869

Merged
merged 3 commits into from
Nov 21, 2024

Conversation

Meissi-jian
Copy link
Contributor

Summary

getifaddrs support returning multiple IPv6 addresses

Impact

getifaddrs

Testing

Tested on vela product

@github-actions github-actions bot added Area: OS Components OS Components issues Size: S The size of the change in this PR is small labels Nov 20, 2024
@nuttxpr
Copy link

nuttxpr commented Nov 20, 2024

[Experimental Bot, please feedback here]

No, this PR does not fully meet the NuttX requirements. While it provides a summary of the change, it lacks crucial details required for proper review.

Here's a breakdown of what's missing:

  • Insufficient Summary: While the summary mentions the change (getifaddrs supporting multiple IPv6 addresses), it lacks the why, how, and related issue references. Why was this change necessary? What part of the code was modified to achieve this? Are there any related NuttX issues?

  • Incomplete Impact Assessment: Simply stating "getifaddrs" doesn't explain the impact. While it implies a change to networking, it doesn't specify:

    • New/Changed Feature: Is this a new feature or a modification to an existing one?
    • User Impact: Will users need to change their code to accommodate this change?
    • Build/Hardware/Documentation/Security/Compatibility Impact: All of these are marked as "NO" by omission, which needs explicit confirmation. Even if there's no impact, stating "NO" explicitly is crucial.
  • Inadequate Testing Information: "Tested on vela product" is far too vague. The requirements ask for:

    • Build Host Details: OS, CPU architecture, compiler, and version.
    • Target Details: Architecture (simulator, RISC-V, ARM, etc.), board, and configuration.
    • Logs Before and After the Change: Actual log output demonstrating the change's effect and proving that it works as intended. This is essential for verifying the fix.

The PR author needs to provide significantly more detail in all sections to meet the NuttX requirements. Without this information, it's difficult to assess the correctness, impact, and stability of the change.

libs/libc/net/lib_getifaddrs.c Outdated Show resolved Hide resolved
libs/libc/net/lib_getifaddrs.c Show resolved Hide resolved
If the pointer is not set to zero, it will erroneously point to itself, resulting in an erroneous loop pointing

Signed-off-by: wangchen <[email protected]>
@xiaoxiang781216 xiaoxiang781216 merged commit b58eaf7 into apache:master Nov 21, 2024
27 checks passed
@Meissi-jian Meissi-jian deleted the getifaddrs branch November 22, 2024 01:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: OS Components OS Components issues Size: S The size of the change in this PR is small
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants