[haiku-commits] Re: r39115 - in haiku/trunk: headers/os/storage headers/private/storage src/add-ons/disk_systems/bfs src/add-ons/disk_systems/intel src/apps/drivesetup ...
- From: Stephan Assmus <superstippi@xxxxxx>
- To: haiku-commits@xxxxxxxxxxxxx
- Date: Sun, 24 Oct 2010 10:50:39 +0200
Hi Bryce,
nice changes, some comments below. :-)
Am 24.10.2010 05:39, schrieb bgroff@xxxxxxxxxx:
Modified: haiku/trunk/headers/os/storage/DiskDeviceDefs.h
===================================================================
--- haiku/trunk/headers/os/storage/DiskDeviceDefs.h 2010-10-24 03:31:34 UTC
(rev 39114)
+++ haiku/trunk/headers/os/storage/DiskDeviceDefs.h 2010-10-24 03:39:19 UTC
(rev 39115)
@@ -148,6 +148,13 @@
B_DISK_DEVICE_JOB_CAN_PAUSE = 0x08,
};
+enum B_PARAMETER_EDITOR_TYPE {
+ B_CREATE_PARAMETER_EDITOR = 0x01,
+ B_INITIALIZE_PARAMETER_EDITOR = 0x04,
+ B_DELETE_PARAMETER_EDITOR = 0x08,
+ B_PROPERTIES_PARAMETER_EDITOR = 0x10
+};
The name of the type should be "BParameterEditorType", IMHO. Also since
these are probably not flags, the values should be more like (0, 1, 2,
3). Or did you envision to have an app ask for
"B_CREATE_PARAMETER_EDITOR | B_INITIALIZE_PARAMETER_EDITOR"?
Modified: haiku/trunk/src/apps/drivesetup/CreateParamsPanel.cpp
===================================================================
--- haiku/trunk/src/apps/drivesetup/CreateParamsPanel.cpp 2010-10-24
03:31:34 UTC (rev 39114)
+++ haiku/trunk/src/apps/drivesetup/CreateParamsPanel.cpp 2010-10-24
03:39:19 UTC (rev 39115)
@@ -265,9 +265,11 @@
)
);
- parent->GetChildCreationParameterEditor(NULL,&fEditor);
- if (fEditor)
+ status_t err =
parent->GetParameterEditor(B_CREATE_PARAMETER_EDITOR,&fEditor);
missing space after comma
+ if (err == B_OK&& fEditor != NULL)
AddChild(fEditor->View());
weird spacing
+ else
+ fEditor = NULL;
BButton* okButton = new BButton(B_TRANSLATE("Create"),
new BMessage(MSG_OK));
@@ -281,3 +283,4 @@
AddToSubset(fWindow);
layout->View()->SetViewColor(ui_color(B_PANEL_BACKGROUND_COLOR));
}
Modified: haiku/trunk/src/apps/drivesetup/InitParamsPanel.cpp
===================================================================
--- haiku/trunk/src/apps/drivesetup/InitParamsPanel.cpp 2010-10-24 03:31:34 UTC
(rev 39114)
+++ haiku/trunk/src/apps/drivesetup/InitParamsPanel.cpp 2010-10-24 03:39:19 UTC
(rev 39115)
@@ -16,6 +16,8 @@
#include<Button.h>
#include<Catalog.h>
#include<ControlLook.h>
+#include<DiskSystemAddOn.h>
+#include<DiskSystemAddOnManager.h>
#include<GroupLayout.h>
#include<GroupLayoutBuilder.h>
#include<Locale.h>
@@ -96,9 +98,21 @@
fOkButton = new BButton(B_TRANSLATE("Initialize"), new
BMessage(MSG_OK));
- partition->GetInitializationParameterEditor(diskSystem.String(),
- &fEditor);
+ DiskSystemAddOnManager* manager = DiskSystemAddOnManager::Default();
+ BDiskSystemAddOn* addOn = manager->GetAddOn(diskSystem);
+ if (addOn) {
+ // put the add-on
+ manager->PutAddOn(addOn);
(Despite my comments below...) Put the addon before using it? Isn't
putting used for the reference count?
+ status_t err =
addOn->GetParameterEditor(B_INITIALIZE_PARAMETER_EDITOR,&fEditor);
+ if (err != B_OK) {
+ fEditor = NULL;
+ }
+ } else {
+ fEditor = NULL;
+ }
+
This change is wrong. The add-ons are supposed to be "hidden" from the
API user. And DriveSetup is only supposed to use the "public" API.
Modified: haiku/trunk/src/apps/drivesetup/PartitionList.cpp
===================================================================
--- haiku/trunk/src/apps/drivesetup/PartitionList.cpp 2010-10-24 03:31:34 UTC
(rev 39114)
+++ haiku/trunk/src/apps/drivesetup/PartitionList.cpp 2010-10-24 03:39:19 UTC
(rev 39115)
@@ -221,7 +221,10 @@
SetField(new BStringField(kUnavailableString),
kFilesystemColumn);
SetField(new BStringField(kUnavailableString),
kVolumeNameColumn);
} else {
- SetField(new BStringField(partition->Type()),
kFilesystemColumn);
+ if (partition->ContainsFileSystem())
+ SetField(new BStringField(partition->Type()),
kFilesystemColumn);
+ else
+ SetField(new BStringField(kUnavailableString),
kFilesystemColumn);
But isn't the user interested in what type flag the partition has? He
may be able to more easily recognize a partition by that.
Modified: haiku/trunk/src/kits/storage/disk_device/Partition.cpp
===================================================================
--- haiku/trunk/src/kits/storage/disk_device/Partition.cpp 2010-10-24
03:31:34 UTC (rev 39114)
+++ haiku/trunk/src/kits/storage/disk_device/Partition.cpp 2010-10-24
03:39:19 UTC (rev 39115)
@@ -1083,13 +1083,13 @@
// GetParameterEditor
status_t
-BPartition::GetParameterEditor(BPartitionParameterEditor** editor)
+BPartition::GetParameterEditor(B_PARAMETER_EDITOR_TYPE type,
+ BPartitionParameterEditor** editor)
{
- BPartition* parent = Parent();
- if (!parent || !fDelegate)
+ if (!fDelegate)
return B_NO_INIT;
- return parent->fDelegate->GetParameterEditor(fDelegate, editor);
+ return fDelegate->GetParameterEditor(type, editor);
}
This may point to a problem in the change overall, which is who is
responsible for what. It looks like you would have to check the type
here already and depending on this, ask either the delegate of the
parent, or your own delegate.
Modified: haiku/trunk/src/kits/storage/disk_device/PartitionDelegate.cpp
===================================================================
--- haiku/trunk/src/kits/storage/disk_device/PartitionDelegate.cpp
2010-10-24 03:31:34 UTC (rev 39114)
+++ haiku/trunk/src/kits/storage/disk_device/PartitionDelegate.cpp
2010-10-24 03:39:19 UTC (rev 39115)
@@ -8,10 +8,8 @@
#include<stdio.h>
[...]
// SetContentParameters
status_t
BPartition::Delegate::SetContentParameters(const char* parameters)
@@ -425,6 +398,12 @@
bool
BPartition::Delegate::CanInitialize(const char* diskSystem) const
{
+ // HACK TO HELP BLANK PARTITION'S BECOME INITIALIZED.
+ if (diskSystem == NULL)
+ return true;
+ if (strlen(diskSystem)< 1)
+ return true;
+
// get the disk system add-on
DiskSystemAddOnManager* manager = DiskSystemAddOnManager::Default();
BDiskSystemAddOn* addOn = manager->GetAddOn(diskSystem);
This looks really weird and in conflict with the intentions of how
things are supposed to work. What's wrong with checking whether an
add-on is available that supports the operation?
Best regards,
-Stephan
Other related posts:
