Refactor: Use symbolic constants for volume header magic numbers #1525
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Replaced hardcoded 0x56455241 ('VERA') with TC_HEADER_MAGIC for better readability and maintainability.
Also replaced 0x5645524142455854 with TC_BOOT_DRIVE_FILTER_EXTENSION_MAGIC and added 'ULL' suffix for 64-bit safety.
There was a problem hiding this comment.
Pull Request Overview
This PR refactors hardcoded volume header magic numbers by replacing them with symbolic constants, improving code readability and maintainability. Key changes include:
- Replacing hardcoded 32‑bit magic numbers with the TC_HEADER_MAGIC_NUMBER constant in multiple source files.
- Removing the redundant definition of TC_BOOT_DRIVE_FILTER_EXTENSION_MAGIC_NUMBER from src/Driver/DriveFilter.h.
- Updating the corresponding comments and ensuring the 64‑bit constant uses the ULL suffix in src/Common/Volumes.h.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/Format/InPlace.c | Updated magic number check to use TC_HEADER_MAGIC_NUMBER |
| src/Driver/DriveFilter.h | Removed redundant TC_BOOT_DRIVE_FILTER_EXTENSION_MAGIC_NUMBER definition |
| src/Driver/DriveFilter.c | Replaced hardcoded magic value with TC_HEADER_MAGIC_NUMBER |
| src/Common/Volumes.h | Introduced symbolic constants with proper ULL suffix for magic numbers |
| src/Common/Volumes.c | Replaced hardcoded magic values with symbolic constants and updated comments |
| src/Common/BootEncryption.cpp | Replaced hardcoded magic value with TC_HEADER_MAGIC_NUMBER |
Comments suppressed due to low confidence (2)
src/Driver/DriveFilter.h:51
- Ensure that the removal of this macro does not break references to TC_BOOT_DRIVE_FILTER_EXTENSION_MAGIC_NUMBER. Verify that all files now include 'src/Common/Volumes.h' where the new definition is provided.
#define TC_BOOT_DRIVE_FILTER_EXTENSION_MAGIC_NUMBER 0x5645524142455854
src/Common/Volumes.c:461
- [nitpick] Consider updating the preceding comment to explicitly mention that 'TC_HEADER_MAGIC_NUMBER' is being used as the volume header identifier, reinforcing the connection between the comment and the symbolic constant.
if (GetHeaderField32 (header, TC_HEADER_OFFSET_MAGIC) != TC_HEADER_MAGIC_NUMBER)
|
Thank you for your contribution. It makes the code clearer. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes
0x56455241('VERA') withTC_HEADER_MAGICfor improved readability and maintainability.TC_BOOT_DRIVE_FILTER_EXTENSION_MAGIC_NUMBER(64-bit"VERABEXT").ULLsuffix for 64-bit constant correctness.