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

coredump:Add a coreudmp api to watch custom variables #13752

Merged
merged 2 commits into from
Oct 1, 2024

Conversation

anjiahao1
Copy link
Contributor

Summary

Add a coreudmp api to watch custom variables

Impact

coredump

Testing

qemu with mps3 an547 syslog coredump

if we want add some variables in coredump
we can use coredump_add_memory_region(&var, sizeof(var))

@github-actions github-actions bot added Area: Documentation Improvements or additions to documentation Area: OS Components OS Components issues Area: BINFMT Size: M The size of the change in this PR is medium labels Sep 30, 2024
@github-actions github-actions bot removed the Area: Documentation Improvements or additions to documentation label Sep 30, 2024
@nuttxpr
Copy link

nuttxpr commented Sep 30, 2024

[Experimental Bot, please feedback here]

No, this Pull Request does not meet the NuttX requirements.

Here's why and how to improve it:

Missing Information:

  • Summary:
    • Why is this change necessary? Clearly state the problem this solves or the benefit it brings. Is it fixing a bug, improving performance, or adding a new feature?
    • What functional part of the code is being changed? Be specific. Mention the files, modules, or subsystems involved (e.g., core dump mechanism, logging subsystem).
    • How does the change exactly work? Provide a technical explanation of the implementation details.
  • Impact:
    • Is a new feature added? Is an existing feature changed? Answer these directly with "YES" or "NO."
    • All other impact categories are incomplete. For each "YES", provide a detailed explanation of the impact and how users/developers might be affected.
  • Testing:
    • Insufficient build host and target information. Provide specifics:
      • Build Host OS and version
      • Build Host CPU architecture
      • Compiler and version
      • Target architecture (e.g., sim:qemu-system-risc32, arm:stm32f4discovery)
      • NuttX configuration used
    • "Testing logs" are not logs. Provide actual output from the system before and after your change to demonstrate the problem and the solution. This could include:
      • Relevant sections from the NuttX build output
      • Serial console output
      • Log files
      • Core dump analysis

Example Improvements:

Summary:

  • Why: "Currently, the NuttX core dump mechanism doesn't allow developers to easily include custom variables for debugging. This makes it difficult to diagnose issues related to specific application data structures."
  • What: "This PR introduces a new API function, coredump_add_memory_region(), to the core dump subsystem. This function allows developers to register specific memory regions containing custom variables to be included in the core dump output."
  • How: "The coredump_add_memory_region() function takes a pointer to the variable and its size as arguments. It adds this information to an internal list maintained by the core dump mechanism. When a core dump is triggered, the registered memory regions are also dumped along with the standard system information."

Impact:

  • Is a new feature added? YES
    • This PR adds a new API function for developers to customize core dump content.
  • Impact on user: YES
    • Users who want to debug applications with custom data structures in core dumps will need to use the new API function in their code.
  • Impact on documentation: YES
    • This PR requires updates to the NuttX documentation to describe the new API function, its usage, and examples.

Testing:

  • Build Host: Ubuntu 20.04, x86_64, GCC 9.3.0
  • Target: sim:qemu-system-risc32, configs/mps3-an547/nsh
  • Testing logs before change:
    • [Show the core dump output WITHOUT your custom variable. Indicate the missing information].
  • Testing logs after change:
    • [Show the core dump output WITH your custom variable included. Demonstrate that the new API function works correctly].

By providing this level of detail, your Pull Request will be much easier to understand, review, and ultimately merge into NuttX.

@xiaoxiang781216 xiaoxiang781216 merged commit f2bdfa4 into apache:master Oct 1, 2024
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: BINFMT Area: OS Components OS Components issues Size: M The size of the change in this PR is medium
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants