From 76016381a7232b852b5a5a898c2bea3f7a820e23 Mon Sep 17 00:00:00 2001 From: Ian Wells Date: Thu, 20 Jan 2022 10:43:15 -0800 Subject: [PATCH 1/4] Confirm os.system commands run os.system('mke2fs') assumes that it's installed (good bet), but also that it works. On Ubuntu it doesn't, not with the FS sizes currently encoded, and that leads to mystery test failures. The straightforward way to fix this is just to raise the error where it happens rather than silently fail; this uses asserts. --- tests/baseclass.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/baseclass.py b/tests/baseclass.py index c599107a..23604ed4 100644 --- a/tests/baseclass.py +++ b/tests/baseclass.py @@ -72,7 +72,7 @@ def setUp(self): os.write(self.fd, b"0") self.f.close() - os.system("mke2fs -F -q %s" % (self.path,)) + assert(os.system("mke2fs -F -q %s" % (self.path,)) == 0) self._device = _ped.device_get(self.path) self._geometry = _ped.Geometry(self._device, 0, self._device.length - 1) @@ -137,7 +137,7 @@ def closest(self, sector, a, b): class RequiresLabeledDevice(RequiresDevice): def setUp(self): RequiresDevice.setUp(self) - os.system("parted -s %s mklabel msdos" % (self.path,)) + assert(os.system("parted -s %s mklabel msdos" % (self.path,)) == 0) # Base class for any test case that requires a _ped.Disk or parted.Disk. class RequiresDisk(RequiresDevice): @@ -161,15 +161,15 @@ def setUp(self): self.mountpoint = None def mkfs(self): - os.system("mkfs.ext2 -F -q %s" % self.path) + assert(os.system("mkfs.ext2 -F -q %s" % self.path) == 0) def doMount(self): self.mountpoint = tempfile.mkdtemp() - os.system("mount -o loop %s %s" % (self.path, self.mountpoint)) + assert(os.system("mount -o loop %s %s" % (self.path, self.mountpoint)) == 0) def removeMountpoint(self): if self.mountpoint and os.path.exists(self.mountpoint): - os.system("umount %s" % self.mountpoint) + assert(os.system("umount %s" % self.mountpoint) == 0) os.rmdir(self.mountpoint) # Base class for any test case that requires a _ped.Partition. From 2591f939d84cf1df12b009be8abe9d040ff18d95 Mon Sep 17 00:00:00 2001 From: Ian Wells Date: Thu, 20 Jan 2022 10:54:48 -0800 Subject: [PATCH 2/4] Increase size of test FS to avoid mke2fs errors mke2fs (on Ubuntu, at least) won't create a 140k filesystem. Up the FS size to 1.4M (which, if nothing else, is old 3.5in HD floppy size). --- tests/baseclass.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/baseclass.py b/tests/baseclass.py index 23604ed4..dc11d0ca 100644 --- a/tests/baseclass.py +++ b/tests/baseclass.py @@ -32,7 +32,7 @@ def setUp(self): self.temp_prefix = "temp-device-" (self.fd, self.path) = tempfile.mkstemp(prefix=self.temp_prefix) self.f = os.fdopen(self.fd) - self.f.seek(140000) + self.f.seek(1400000) os.write(self.fd, b"0") def removeTempDevice(self): @@ -68,7 +68,7 @@ def setUp(self): self.temp_prefix = "temp-device-" (self.fd, self.path,) = tempfile.mkstemp(prefix=self.temp_prefix) self.f = os.fdopen(self.fd) - self.f.seek(140000) + self.f.seek(1400000) os.write(self.fd, b"0") self.f.close() From 9a4036504c37540f5def8046fca5108e1c6cb079 Mon Sep 17 00:00:00 2001 From: Ian Wells Date: Thu, 20 Jan 2022 10:57:09 -0800 Subject: [PATCH 3/4] Adjust formatting for percentage based results Having changed the size of the FS formats, it's noticeable that the %-based numbers were coming out with a 0.01% error at 2dp accuracy. It appears that this is because we were using %f[:4] (which floor()s at 0.01) rather than rounding to nearest. Change code to use %.2f which does the rounding. --- tests/test__ped_device.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/test__ped_device.py b/tests/test__ped_device.py index 33b250e1..81384c77 100755 --- a/tests/test__ped_device.py +++ b/tests/test__ped_device.py @@ -243,7 +243,7 @@ def runTest(self): class UnitFormatCustomByteTestCase(RequiresDevice): def setUp(self): RequiresDevice.setUp(self) - pr = "%f" % (47.0 / self._device.unit_get_size(_ped.UNIT_PERCENT),) + pr = "%.2f" % (47.0 / self._device.unit_get_size(_ped.UNIT_PERCENT),) self.pairs = [(_ped.UNIT_SECTOR, '0s',), (_ped.UNIT_BYTE, '47B',), (_ped.UNIT_KILOBYTE, '0.05kB',), @@ -253,7 +253,7 @@ def setUp(self): (_ped.UNIT_COMPACT, '47.0B',), (_ped.UNIT_CYLINDER, '0cyl',), (_ped.UNIT_CHS, '0,0,0',), - (_ped.UNIT_PERCENT, pr[:4] + "%",), + (_ped.UNIT_PERCENT, pr + "%",), (_ped.UNIT_KIBIBYTE, '0.05kiB',), (_ped.UNIT_MEBIBYTE, '0.00MiB',), (_ped.UNIT_GIBIBYTE, '0.00GiB',), @@ -267,7 +267,7 @@ def runTest(self): class UnitFormatByteTestCase(RequiresDevice): def setUp(self): RequiresDevice.setUp(self) - pr = "%f" % (47.0 / self._device.unit_get_size(_ped.UNIT_PERCENT),) + pr = "%.2f" % (47.0 / self._device.unit_get_size(_ped.UNIT_PERCENT),) self._initialDefault = _ped.unit_get_default() self.pairs = [(_ped.UNIT_SECTOR, '0s',), (_ped.UNIT_BYTE, '47B',), @@ -278,7 +278,7 @@ def setUp(self): (_ped.UNIT_COMPACT, '47.0B',), (_ped.UNIT_CYLINDER, '0cyl',), (_ped.UNIT_CHS, '0,0,0',), - (_ped.UNIT_PERCENT, pr[:4] + "%",), + (_ped.UNIT_PERCENT, pr + "%",), (_ped.UNIT_KIBIBYTE, '0.05kiB',), (_ped.UNIT_MEBIBYTE, '0.00MiB',), (_ped.UNIT_GIBIBYTE, '0.00GiB',), @@ -298,7 +298,7 @@ def setUp(self): RequiresDevice.setUp(self) sector_size = self._device.sector_size size = self._device.unit_get_size(_ped.UNIT_PERCENT) - pr = "%f" % ((47.0 * sector_size) / size,) + pr = "%.2f" % ((47.0 * sector_size) / size,) self.pairs = [(_ped.UNIT_SECTOR, '47s',), (_ped.UNIT_BYTE, '24064B',), (_ped.UNIT_KILOBYTE, '24.1kB',), @@ -308,7 +308,7 @@ def setUp(self): (_ped.UNIT_COMPACT, '24.1kB',), (_ped.UNIT_CYLINDER, '0cyl',), (_ped.UNIT_CHS, '0,1,15',), - (_ped.UNIT_PERCENT, pr[:4] + "%",), + (_ped.UNIT_PERCENT, pr + "%",), (_ped.UNIT_KIBIBYTE, '23.5kiB',), (_ped.UNIT_MEBIBYTE, '0.02MiB',), (_ped.UNIT_GIBIBYTE, '0.00GiB',), @@ -324,7 +324,7 @@ def setUp(self): RequiresDevice.setUp(self) sector_size = self._device.sector_size size = self._device.unit_get_size(_ped.UNIT_PERCENT) - pr = "%f" % ((47.0 * sector_size) / size,) + pr = "%.2f" % ((47.0 * sector_size) / size,) self._initialDefault = _ped.unit_get_default() self.pairs = [(_ped.UNIT_SECTOR, '47s',), (_ped.UNIT_BYTE, '24064B',), @@ -335,7 +335,7 @@ def setUp(self): (_ped.UNIT_COMPACT, '24.1kB',), (_ped.UNIT_CYLINDER, '0cyl',), (_ped.UNIT_CHS, '0,1,15',), - (_ped.UNIT_PERCENT, pr[:4] + "%",), + (_ped.UNIT_PERCENT, pr + "%",), (_ped.UNIT_KIBIBYTE, '23.5kiB',), (_ped.UNIT_MEBIBYTE, '0.02MiB',), (_ped.UNIT_GIBIBYTE, '0.00GiB',), From 71e8398fe62c78cf2baab222784a142dbb6e4524 Mon Sep 17 00:00:00 2001 From: Ian Wells Date: Thu, 20 Jan 2022 11:04:40 -0800 Subject: [PATCH 4/4] Adjust code to deal with compile warnings Recent Python versions return 'const char *' to PyUnicode_asUTF8. This can give compile errors (I was using Ubuntu 20.04's default build env). Some code changes had been made to case to 'char *', but it's dangerous to discard the const and apparently insufficient to address the warning. Update the code to make use 'const char *' variables where relevant. The warnings go away. Functionality is unaffected. --- src/pyconstraint.c | 12 ++++++------ src/pydevice.c | 6 +++--- src/pydisk.c | 4 ++-- src/pyfilesys.c | 6 +++--- src/pygeom.c | 4 ++-- src/pytimer.c | 8 +++++--- 6 files changed, 21 insertions(+), 19 deletions(-) diff --git a/src/pyconstraint.c b/src/pyconstraint.c index 7845eab8..8a8ce0ab 100644 --- a/src/pyconstraint.c +++ b/src/pyconstraint.c @@ -95,25 +95,25 @@ PyObject *_ped_Constraint_richcompare(_ped_Constraint *a, PyObject *b, int op) { PyObject *_ped_Constraint_str(_ped_Constraint *self) { char *ret = NULL; - char *start_align = NULL, *end_align = NULL; - char *start_range = NULL, *end_range = NULL; + const char *start_align = NULL, *end_align = NULL; + const char *start_range = NULL, *end_range = NULL; - start_align = (char *) PyUnicode_AsUTF8(_ped_Alignment_Type_obj.tp_repr(self->start_align)); + start_align = PyUnicode_AsUTF8(_ped_Alignment_Type_obj.tp_repr(self->start_align)); if (start_align == NULL) { return NULL; } - end_align = (char *) PyUnicode_AsUTF8(_ped_Alignment_Type_obj.tp_repr(self->end_align)); + end_align = PyUnicode_AsUTF8(_ped_Alignment_Type_obj.tp_repr(self->end_align)); if (end_align == NULL) { return NULL; } - start_range = (char *) PyUnicode_AsUTF8(_ped_Geometry_Type_obj.tp_repr(self->start_range)); + start_range = PyUnicode_AsUTF8(_ped_Geometry_Type_obj.tp_repr(self->start_range)); if (start_range == NULL) { return NULL; } - end_range = (char *) PyUnicode_AsUTF8(_ped_Geometry_Type_obj.tp_repr(self->end_range)); + end_range = PyUnicode_AsUTF8(_ped_Geometry_Type_obj.tp_repr(self->end_range)); if (end_range == NULL) { return NULL; } diff --git a/src/pydevice.c b/src/pydevice.c index 20f492e4..6c09d08b 100644 --- a/src/pydevice.c +++ b/src/pydevice.c @@ -186,14 +186,14 @@ PyObject *_ped_Device_richcompare(_ped_Device *a, PyObject *b, int op) { PyObject *_ped_Device_str(_ped_Device *self) { char *ret = NULL; - char *hw_geom = NULL, *bios_geom = NULL; + const char *hw_geom = NULL, *bios_geom = NULL; - hw_geom = (char *) PyUnicode_AsUTF8(_ped_CHSGeometry_Type_obj.tp_repr(self->hw_geom)); + hw_geom = PyUnicode_AsUTF8(_ped_CHSGeometry_Type_obj.tp_repr(self->hw_geom)); if (hw_geom == NULL) { return NULL; } - bios_geom = (char *) PyUnicode_AsUTF8(_ped_CHSGeometry_Type_obj.tp_repr(self->bios_geom)); + bios_geom = PyUnicode_AsUTF8(_ped_CHSGeometry_Type_obj.tp_repr(self->bios_geom)); if (bios_geom == NULL) { return NULL; } diff --git a/src/pydisk.c b/src/pydisk.c index 8856f8eb..a67f43fd 100644 --- a/src/pydisk.c +++ b/src/pydisk.c @@ -98,7 +98,7 @@ PyObject *_ped_Partition_richcompare(_ped_Partition *a, PyObject *b, int op) { PyObject *_ped_Partition_str(_ped_Partition *self) { char *ret = NULL; - char *disk = NULL, *fs_type = NULL, *geom = NULL; + const char *disk = NULL, *fs_type = NULL, *geom = NULL; disk = (char *) PyUnicode_AsUTF8(_ped_Disk_Type_obj.tp_repr(self->disk)); if (disk == NULL) { @@ -345,7 +345,7 @@ PyObject *_ped_Disk_richcompare(_ped_Disk *a, PyObject *b, int op) { PyObject *_ped_Disk_str(_ped_Disk *self) { char *ret = NULL; - char *dev = NULL, *type = NULL; + const char *dev = NULL, *type = NULL; dev = (char *) PyUnicode_AsUTF8(_ped_Device_Type_obj.tp_repr(self->dev)); if (dev == NULL) { diff --git a/src/pyfilesys.c b/src/pyfilesys.c index 53abd905..637a61d1 100644 --- a/src/pyfilesys.c +++ b/src/pyfilesys.c @@ -171,14 +171,14 @@ PyObject *_ped_FileSystem_richcompare(_ped_FileSystem *a, PyObject *b, int op) { PyObject *_ped_FileSystem_str(_ped_FileSystem *self) { char *ret = NULL; - char *type = NULL, *geom = NULL; + const char *type = NULL, *geom = NULL; - type = (char *) PyUnicode_AsUTF8(_ped_FileSystem_Type_obj.tp_repr(self->type)); + type = PyUnicode_AsUTF8(_ped_FileSystem_Type_obj.tp_repr(self->type)); if (type == NULL) { return NULL; } - geom = (char *) PyUnicode_AsUTF8(_ped_Geometry_Type_obj.tp_repr(self->geom)); + geom = PyUnicode_AsUTF8(_ped_Geometry_Type_obj.tp_repr(self->geom)); if (geom == NULL) { return NULL; } diff --git a/src/pygeom.c b/src/pygeom.c index 811e6f5b..a4263c01 100644 --- a/src/pygeom.c +++ b/src/pygeom.c @@ -86,9 +86,9 @@ PyObject *_ped_Geometry_richcompare(_ped_Geometry *a, PyObject *b, int op) { PyObject *_ped_Geometry_str(_ped_Geometry *self) { char *ret = NULL; - char *dev = NULL; + const char *dev = NULL; - dev = (char *) PyUnicode_AsUTF8(_ped_Device_Type_obj.tp_repr(self->dev)); + dev = PyUnicode_AsUTF8(_ped_Device_Type_obj.tp_repr(self->dev)); if (dev == NULL) { return NULL; } diff --git a/src/pytimer.c b/src/pytimer.c index 6cf1c95e..11f8a8e0 100644 --- a/src/pytimer.c +++ b/src/pytimer.c @@ -164,19 +164,21 @@ int _ped_Timer_set(_ped_Timer *self, PyObject *value, void *closure) { return -1; } } else if (!strcmp(member, "state_name")) { - self->state_name = (char *) PyUnicode_AsUTF8(value); + const char *state_name = PyUnicode_AsUTF8(value); if (PyErr_Occurred()) { return -1; } /* self->state_name now points to the internal buffer of a PyUnicode obj * which may be freed when its refcount drops to zero, so strdup it. */ - if (self->state_name) { - self->state_name = strdup(self->state_name); + if (state_name) { + self->state_name = strdup(state_name); if (!self->state_name) { PyErr_NoMemory(); return -2; } + } else { + self->state_name = NULL; } } else { PyErr_Format(PyExc_AttributeError, "_ped.Timer object has no attribute %s", member);