part: Fix partition ID validation

[vojtechtrefny]

Follow up for a42600f. We want to be able to set all valid IDs even those unknown to libfdisk.

Summary by CodeRabbit

• Bug Fixes

  • Tightened partition ID validation: now accepts additional valid IDs (e.g., 0xc8) and rejects negative/out-of-range values.
  • Fixed an issue that could incorrectly reject certain partition types for MSDOS/MBR layouts, reducing false errors when applying types.

• Tests

  • Updated test suite to cover the new partition ID rules and the negative-ID error case.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: ed37a127-ce82-401a-8dfd-19288345d933

📥 Commits

Reviewing files that changed from the base of the PR and between a7ab00f and f9e0824.

📒 Files selected for processing (2)

• src/plugins/part.c
• tests/part_test.py

🚧 Files skipped from review as they are similar to previous changes (1)

• tests/part_test.py

---

📝 Walkthrough

Walkthrough

Partition id parsing in src/plugins/part.c now parses to a signed 64-bit integer, enforces 1 ≤ id ≤ 0xFF, uses g_ascii_strtoll, and removes the previous unknown-GPT-type validation block. Tests in tests/part_test.py were extended to cover valid high ids and negative-id error handling.

Changes

Cohort / File(s)	Summary
Implementation src/plugins/part.c	Changed partition id variable to gint64, switched parsing to g_ascii_strtoll, tightened validation to require 1 ≤ id ≤ 0xFF and full-consumption of input; removed the unknown-GPT-part-type validation branch for MSDOS/MBR.
Tests tests/part_test.py	Added/updated tests to assert that 0xc8 is accepted and persisted, and that negative ids like "-1" raise GLib.GError; retained existing invalid-id test cases.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% 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
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'part: Fix partition ID validation' directly summarizes the main change: fixing partition ID validation to allow all valid IDs including unrecognized ones.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

🧹 Nitpick comments (1)

src/plugins/part.c (1)

1728-1734: Consider using gint64 for clearer signed comparison.

The variable part_id_int is declared as guint64, but g_ascii_strtoll returns gint64. While the code works correctly (negative inputs like "-1" wrap to large unsigned values caught by > 0xFF), the comparison part_id_int <= 0 on an unsigned type is effectively just part_id_int == 0.

Using a signed type would make the intent clearer:

♻️ Suggested improvement for type consistency

-    guint64 part_id_int = 0;
+    gint64 part_id_int = 0;
 
     /* check if part type/id is valid for MBR */
     if (table_type == BD_PART_TABLE_MSDOS) {
         part_id_int = g_ascii_strtoll (type_str, &endptr, 0);
 
-        if (part_id_int <= 0 || part_id_int > 0xFF || endptr == NULL || *endptr != '\0') {
+        if (part_id_int < 1 || part_id_int > 0xFF || endptr == NULL || *endptr != '\0') {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/plugins/part.c` around lines 1728 - 1734, part_id_int is declared as
guint64 but assigned from g_ascii_strtoll (which returns gint64), making the
signed comparison part_id_int <= 0 misleading; change the declaration of
part_id_int to gint64 (or another signed 64-bit type) so negative parse results
are represented correctly, then keep the existing validation logic that checks
if part_id_int <= 0 || part_id_int > 0xFF || endptr == NULL || *endptr != '\0'
inside the BD_PART_TABLE_MSDOS branch (symbols: part_id_int, g_ascii_strtoll,
type_str, endptr, BD_PART_TABLE_MSDOS).

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/plugins/part.c`:
- Around line 1728-1734: part_id_int is declared as guint64 but assigned from
g_ascii_strtoll (which returns gint64), making the signed comparison part_id_int
<= 0 misleading; change the declaration of part_id_int to gint64 (or another
signed 64-bit type) so negative parse results are represented correctly, then
keep the existing validation logic that checks if part_id_int <= 0 ||
part_id_int > 0xFF || endptr == NULL || *endptr != '\0' inside the
BD_PART_TABLE_MSDOS branch (symbols: part_id_int, g_ascii_strtoll, type_str,
endptr, BD_PART_TABLE_MSDOS).

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 70997a53-f8ab-4a47-8bec-2c207debe14c

📥 Commits

Reviewing files that changed from the base of the PR and between 80490b9 and a7ab00f.

📒 Files selected for processing (2)

• src/plugins/part.c
• tests/part_test.py

[tbzatek]

Even better!