Skip to content

Fix uninitialized platformStyle member in AddressBookPage#1815

Open
reubenyap wants to merge 2 commits intofiroorg:masterfrom
reubenyap:fix-addressbookpage-uninit-platformstyle
Open

Fix uninitialized platformStyle member in AddressBookPage#1815
reubenyap wants to merge 2 commits intofiroorg:masterfrom
reubenyap:fix-addressbookpage-uninit-platformstyle

Conversation

@reubenyap
Copy link
Copy Markdown
Member

@reubenyap reubenyap commented Apr 19, 2026

User description

Summary

The AddressBookPage constructor in src/qt/addressbookpage.cpp accepts a const PlatformStyle *platformStyle parameter that shadows the class member of the same name declared at src/qt/addressbookpage.h:66. The constructor body uses the parameter for icon setup but never assigns it to the member, so this->platformStyle remained uninitialized (indeterminate pointer value).

The uninitialized member is later read in on_newAddress_clicked when constructing CreateSparkNamePage(platformStyle, this). Reading an uninitialized pointer is undefined behavior. Today it happens not to crash because CreateSparkNamePage's constructor ignores the parameter, but any future change that actually uses it would expose the bug.

Fix: add platformStyle(platformStyle) to the constructor initializer list. One-line change, matching the pattern already used in src/qt/manualmintdialog.cpp.

Test plan

  • Build firo-qt and confirm it compiles without new warnings
  • Open the address book from the GUI and click "New" to exercise on_newAddress_clicked; verify the Spark name dialog path still works

CodeAnt-AI Description

Fix address book button state and Spark name dialog setup

What Changed

  • The address book now keeps the Delete button disabled when a Spark name is selected, matching its hidden state and the delete action.
  • The address book now stores the platform style pointer it receives, so opening the New Spark name dialog uses a valid value.

Impact

✅ Fewer confusing delete-button states
✅ Safer Spark name dialog opening
✅ Lower risk of address book crashes

🔄 Retrigger CodeAnt AI Review

Details

💡 Usage Guide

Checking Your Pull Request

Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.

Talking to CodeAnt AI

Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:

@codeant-ai ask: Your question here

This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.

Example

@codeant-ai ask: Can you suggest a safer alternative to storing this secret?

Preserve Org Learnings with CodeAnt

You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:

@codeant-ai: Your feedback here

This helps CodeAnt AI learn and adapt to your team's coding style and standards.

Example

@codeant-ai: Do not flag unused imports.

Retrigger review

Ask CodeAnt AI to review the PR again, by typing:

@codeant-ai: review

Check Your Repository Health

To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.

…-name tabs

In AddressBookPage::selectionChanged, the delete button was unconditionally
re-enabled on the Sending tab even when the current address type is a spark
name, leaving it enabled=true but visible=false. chooseAddressType sets
enabled=false when switching to a spark-name tab, but the next row-click on
that tab flipped it back.

Harmless today (hidden buttons can't be clicked), but the state is
inconsistent with deleteAction and the button's visibility, and would trip
up a future refactor that stops hiding the button.

Gate setEnabled on fSparkNames so the three flags move together.
The AddressBookPage constructor accepted a platformStyle parameter
that shadowed the member of the same name, so the member was never
initialized. Reading it later (e.g. in on_newAddress_clicked when
constructing CreateSparkNamePage) was undefined behavior. Add the
member to the initializer list so it holds the passed-in pointer.
@codeant-ai
Copy link
Copy Markdown

codeant-ai Bot commented Apr 19, 2026

CodeAnt AI is reviewing your PR.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 19, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: ccc9a04c-e1a3-46ac-ae0b-4de8b24584bc

📥 Commits

Reviewing files that changed from the base of the PR and between 4943a33 and 4cdf54b.

📒 Files selected for processing (1)
  • src/qt/addressbookpage.cpp

Summary by CodeRabbit

Release Notes

  • Bug Fixes
    • Fixed deletion control behavior in the address book to properly disable deletion for specific address types, ensuring safer address management.

Walkthrough

The AddressBookPage constructor now properly initializes the platformStyle member variable. Additionally, the delete address button in SendingTab is disabled for SparkName addresses instead of being always enabled, preventing deletion of this special address type.

Changes

Cohort / File(s) Summary
AddressBookPage Constructor and Selection Logic
src/qt/addressbookpage.cpp
Added platformStyle member initialization in constructor. Modified selectionChanged() to disable the delete address button when the current address is a SparkName (by checking !fSparkNames instead of always enabling).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main fix: initializing the uninitialized platformStyle member in AddressBookPage's constructor, which is the primary bug addressed in the PR.
Description check ✅ Passed The PR description includes the mandatory PR intention section, clearly explaining what the change fixes (uninitialized platformStyle member and delete button state sync), the problem it solves, and a test plan with specific steps.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codeant-ai codeant-ai Bot added the size:XS This PR changes 0-9 lines, ignoring generated files label Apr 19, 2026
@codeant-ai
Copy link
Copy Markdown

codeant-ai Bot commented Apr 19, 2026

CodeAnt AI finished reviewing your PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XS This PR changes 0-9 lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants