[haiku-commits] Re: r34967 - in haiku/trunk: headers/private/kernel/fs src/system/kernel/device_manager src/system/kernel/disk_device_manager src/tests/add-ons/kernel/kernelland_emu
- From: Ingo Weinhold <ingo_weinhold@xxxxxx>
- To: haiku-commits@xxxxxxxxxxxxx
- Date: Sat, 09 Jan 2010 13:02:17 +0100
On 2010-01-09 at 04:55:39 [+0100], mmlr@xxxxxxxx wrote:
> Author: mmlr
> Date: 2010-01-09 04:55:38 +0100 (Sat, 09 Jan 2010)
> New Revision: 34967
> Changeset: http://dev.haiku-os.org/changeset/34967/haiku
> Ticket: http://dev.haiku-os.org/ticket/4587
>
> Modified:
> haiku/trunk/headers/private/kernel/fs/devfs.h
> haiku/trunk/src/system/kernel/device_manager/devfs.cpp
> haiku/trunk/src/system/kernel/disk_device_manager/KPartition.cpp
> haiku/trunk/src/tests/add-ons/kernel/kernelland_emu/device_manager.cpp
> Log:
> Reverted the part of r31520 that made devfs_unpublish_partition() take a raw
> device path + child partition name. When a "raw" device is unpublished the
> node
> removal notification triggers the partition and child partitions to be
> unpublished/removed. Since in that case the "raw" node is already
> unpublished
> trying to resolve it in devfs_unpublish_partition() again to unpublish the
> child
> partitions would fail, leaving the child partition nodes behind. When a new
> raw
> device would then become available publishing its partitions would fail
> because
> of these left behind nodes, causing bug #4587. Seeing that this code is more
> compact and straight forward anyway I don't quite see why it was changed in
> the
> first place.
Well more compact in the devfs, less compact in KPartition. Most importantly
though, UnpublishPartition() wouldn't need memory and thus be fail-safe,
which is always a good thing in code that frees resources.
[...]
> Modified: haiku/trunk/src/system/kernel/disk_device_manager/KPartition.cpp
> ===================================================================
> --- haiku/trunk/src/system/kernel/disk_device_manager/KPartition.cpp
> 2010-01-09 02:21:22 UTC (rev 34966)
> +++ haiku/trunk/src/system/kernel/disk_device_manager/KPartition.cpp
> 2010-01-09 03:55:38 UTC (rev 34967)
> @@ -232,9 +232,16 @@
> if (!fPublishedName)
> return B_OK;
>
> - status_t error = devfs_unpublish_partition(Device()->Path(),
> - fPublishedName);
> + // get the path
> + KPath path;
> + status_t error = GetPath(&path);
This could fail due to memory shortage, and...
> if (error != B_OK) {
> + dprintf("KPartition::UnpublishDevice(): Failed to get path for "
> + "partition %ld: %s\n", ID(), strerror(error));
> + }
> +
> + error = devfs_unpublish_partition(path.Path());
... this would be called with an empty path.
> + if (error != B_OK) {
> dprintf("KPartition::UnpublishDevice(): Failed to unpublish
> partition "
> "%ld: %s\n", ID(), strerror(error));
> }
CU, Ingo
Other related posts:
