Hi folks!
So, I was trying to fix pyparted in Fedora Rawhide, and it kinda
snowballed to a point where I'm staring at the _FIRST_ and _LAST_
macros in disk.h and wondering how they're ever supposed to work.
There are three places in disk.h that work basically like this:
enum _PedDiskTypeFeature {
PED_DISK_TYPE_EXTENDED=1, /**< supports extended partitions
*/
PED_DISK_TYPE_PARTITION_NAME=2, /**< supports partition names */
PED_DISK_TYPE_PARTITION_TYPE_ID=4, /**< supports partition type-ids
*/
PED_DISK_TYPE_PARTITION_TYPE_UUID=8, /**< supports partition
type-uuids */
};
#define PED_DISK_TYPE_FIRST_FEATURE PED_DISK_TYPE_EXTENDED
#define PED_DISK_TYPE_LAST_FEATURE PED_DISK_TYPE_PARTITION_TYPE_UUID
pyparted tries to use these. For instance, in its C module code, it
does a lot of this:
#if PED_PARTITION_LAST_FLAG > 20
PyModule_AddIntConstant(m, "PARTITION_LINUX_HOME",
PED_PARTITION_LINUX_HOME);
#endif
However, this does not actually work, because of the problem explained
here: https://stackoverflow.com/questions/34677148
enums are handled by the compiler, so at preprocessor time, all the
enum values effectively evaluate to 0. That check will never 'succeed',
and pyparted will never actually add that constant. This is easy to
check: build pyparted 3.12.0 against current parted and check if the
`_ped` module you get actually has a `PARTITION_LINUX_HOME` attribute.
It won't. (You can even check this in Fedora Rawhide's current
pyparted...)
Up until 3.12.0, pyparted did this instead, as the stackoverflow thread
suggests:
if (PED_PARTITION_LAST_FLAG > 20) {
PyModule_AddIntConstant(m, "PARTITION_LINUX_HOME",
PED_PARTITION_LINUX_HOME);
}
However, it turns out *that* doesn't really work either. If you
actually try and build that code with an older parted, the build fails.
I wrote a similar check into my patch to try and make pyparted work
with parted 3.5:
if (PED_DISK_TYPE_LAST_FEATURE > 2) {
PyModule_AddIntConstant(m, "DISK_TYPE_PARTITION_TYPE_ID",
PED_DISK_TYPE_PARTITION_TYPE_ID);
PyModule_AddIntConstant(m, "DISK_TYPE_PARTITION_TYPE_UUID",
PED_DISK_TYPE_PARTITION_TYPE_UUID);
}
and that works great building against 3.5, but if you try and build
that against parted 3.4, it fails - the compiler still wants
PED_DISK_TYPE_PARTITION_TYPE_ID and PED_DISK_TYPE_PARTITION_TYPE_UUID
to be defined, even though they're in the if block:
src/_pedmodule.c: In function ‘PyInit__ped’:
src/_pedmodule.c:689:63: error: ‘PED_DISK_TYPE_PARTITION_TYPE_ID’ undeclared
(first use in this function); did you mean ‘PED_DISK_TYPE_PARTITION_NAME’?
689 | PyModule_AddIntConstant(m, "DISK_TYPE_PARTITION_TYPE_ID",
PED_DISK_TYPE_PARTITION_TYPE_ID);
|
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
|
PED_DISK_TYPE_PARTITION_NAME
src/_pedmodule.c:689:63: note: each undeclared identifier is reported only once
for each function it appears in
src/_pedmodule.c:690:65: error: ‘PED_DISK_TYPE_PARTITION_TYPE_UUID’ undeclared
(first use in this function); did you mean ‘PED_DISK_TYPE_PARTITION_NAME’?
690 | PyModule_AddIntConstant(m, "DISK_TYPE_PARTITION_TYPE_UUID",
PED_DISK_TYPE_PARTITION_TYPE_UUID);
|
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
|
PED_DISK_TYPE_PARTITION_NAME
so if you can't use these _LAST_ macros at preprocessor time or at
compile/run time, how *can* you ever use them? Am I missing something?
If I'm not missing anything...see the attached patch, I guess?
I'm not subscribed to the list, so please CC me on replies - thanks!
--
Adam Williamson
Fedora QA
IRC: adamw | Twitter: adamw_ha
https://www.happyassassin.net
From 6ce543f4999f21b363865bf845543326ba91406b Mon Sep 17 00:00:00 2001
From: Adam Williamson <[email protected]>
Date: Mon, 20 Jun 2022 12:07:14 -0700
Subject: [PATCH] Set _FIRST_ and _LAST_ macro values in disk.h directly
I may be missing something, but it seems impossible to use these
_FIRST_ and _LAST_ macros in practice as they are currently set.
You cannot use them at preprocessor time, because they will
always evaluate to 0 at that time, because the preprocessor does
not handle enums:
https://stackoverflow.com/questions/34677148
and you cannot really use them at build time, because if you do
something like this:
if (PED_PARTITION_LAST_FLAG > 17) {
PyModule_AddIntConstant(m, "PARTITION_ESP", PED_PARTITION_ESP);
}
the compiler still throws an undeclared identifier error if you
actually try and build the code against an older parted.
Unless anyone can suggest a way to use these properly as-is,
this is the best idea I can come up with: we just have to define
the _FIRST_ and _LAST_ values directly, and remember to keep
them in line when adding new entries to the enums.
Signed-off-by: Adam Williamson <[email protected]>
---
include/parted/disk.in.h | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/include/parted/disk.in.h b/include/parted/disk.in.h
index 672c4ee..fedcee9 100644
--- a/include/parted/disk.in.h
+++ b/include/parted/disk.in.h
@@ -47,8 +47,8 @@ enum _PedDiskFlag {
/* This flag controls whether the boot flag of a GPT PMBR is set */
PED_DISK_GPT_PMBR_BOOT=2,
};
-#define PED_DISK_FIRST_FLAG PED_DISK_CYLINDER_ALIGNMENT
-#define PED_DISK_LAST_FLAG PED_DISK_GPT_PMBR_BOOT
+#define PED_DISK_FIRST_FLAG 1
+#define PED_DISK_LAST_FLAG 2
/**
* Partition types
@@ -88,8 +88,8 @@ enum _PedPartitionFlag {
PED_PARTITION_BLS_BOOT=20,
PED_PARTITION_LINUX_HOME=21,
};
-#define PED_PARTITION_FIRST_FLAG PED_PARTITION_BOOT
-#define PED_PARTITION_LAST_FLAG PED_PARTITION_LINUX_HOME
+#define PED_PARTITION_FIRST_FLAG 1
+#define PED_PARTITION_LAST_FLAG 21
enum _PedDiskTypeFeature {
PED_DISK_TYPE_EXTENDED=1, /**< supports extended partitions */
@@ -97,8 +97,8 @@ enum _PedDiskTypeFeature {
PED_DISK_TYPE_PARTITION_TYPE_ID=4, /**< supports partition type-ids */
PED_DISK_TYPE_PARTITION_TYPE_UUID=8, /**< supports partition type-uuids */
};
-#define PED_DISK_TYPE_FIRST_FEATURE PED_DISK_TYPE_EXTENDED
-#define PED_DISK_TYPE_LAST_FEATURE PED_DISK_TYPE_PARTITION_TYPE_UUID
+#define PED_DISK_TYPE_FIRST_FEATURE 1
+#define PED_DISK_TYPE_LAST_FEATURE 8
struct _PedDisk;
struct _PedPartition;
--
2.36.1
