[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: "Michael Lotz" <mmlr@xxxxxxxx>
- To: haiku-commits@xxxxxxxxxxxxx
- Date: Sat, 09 Jan 2010 13:53:22 +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:
>> ..
> > 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.
I understand that this is desireable, but I don't see how this could
work otherwise. It requires the raw device node, which is the one being
removed first. Of course logic could be implemented to unpublish the
other nodes manually before the raw node is removed, but I think that's
not really nicer than this.
> [...]
> > 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.
Yup, missing return in the error check. Going to fix that.
Regards
Michael
Other related posts:
