[haiku-commits] Re: haiku: hrev50491 - src/preferences/keymap

  • From: Stephan Aßmus <superstippi@xxxxxx>
  • To: haiku-commits@xxxxxxxxxxxxx
  • Date: Wed, 17 Aug 2016 10:02:28 +0200

Hi John,

Am 17.08.2016 um 00:02 schrieb jscipione@xxxxxxxxx:

1 file changed, 22 insertions(+), 14 deletions(-)
src/preferences/keymap/KeymapWindow.cpp | 36 ++++++++++++++++++-----------

############################################################################

Commit:      14698e043233c4fb0f783993d0c874c8241eff77
URL:         http://cgit.haiku-os.org/haiku/commit/?id=14698e043233
Author:      John Scipione <jscipione@xxxxxxxxx>
Date:        Tue Aug 16 21:55:36 2016 UTC

Keymap: Check return value of get_ref_for_path

if not B_OK, create BFilePanels with default directory

This should never happen but better than crashing on a NULL ref

----------------------------------------------------------------------------

diff --git a/src/preferences/keymap/KeymapWindow.cpp 
b/src/preferences/keymap/KeymapWindow.cpp
index a8a8459..cb93cae 100644
--- a/src/preferences/keymap/KeymapWindow.cpp
+++ b/src/preferences/keymap/KeymapWindow.cpp
@@ -133,17 +133,23 @@ KeymapWindow::KeymapWindow()
      path.Append("Keymap");

      entry_ref ref;
-     get_ref_for_path(path.Path(), &ref);
-
-     BDirectory userKeymapsDir(&ref);
-     if (userKeymapsDir.InitCheck() != B_OK)
-             create_directory(path.Path(), S_IRWXU | S_IRWXG | S_IRWXO);
-
-     BMessenger messenger(this);
-     fOpenPanel = new BFilePanel(B_OPEN_PANEL, &messenger, &ref,
-             B_FILE_NODE, false, NULL);
-     fSavePanel = new BFilePanel(B_SAVE_PANEL, &messenger, &ref,
-             B_FILE_NODE, false, NULL);
+     if (get_ref_for_path(path.Path(), &ref) == B_OK) {
+             BDirectory userKeymapsDir(&ref);
+             if (userKeymapsDir.InitCheck() != B_OK)
+                     create_directory(path.Path(), S_IRWXU | S_IRWXG | 
S_IRWXO);
+
+             BMessenger messenger(this);
+             fOpenPanel = new BFilePanel(B_OPEN_PANEL, &messenger, &ref,
+                     B_FILE_NODE, false, NULL);
+             fSavePanel = new BFilePanel(B_SAVE_PANEL, &messenger, &ref,
+                     B_FILE_NODE, false, NULL);
+     } else {
+             BMessenger messenger(this);
+             fOpenPanel = new BFilePanel(B_OPEN_PANEL, &messenger, NULL,
+                     B_FILE_NODE, false, NULL);
+             fSavePanel = new BFilePanel(B_SAVE_PANEL, &messenger, NULL,
+                     B_FILE_NODE, false, NULL);
+     }

This actually looks worse. First of all, there is no NULL ref involved, even 
when get_ref_for_path() fails. What this method appears to intend is that the 
directory should be created if it does not already exist. The previous version 
of the code was OK up until create_directory(). It doesn’t need to check the 
return code of get_ref_for_path(), since it checked BDirectory::InitCheck() 
instead. Where it was wrong is that it used „ref“ then anyway. It needs to 
re-get the ref in that case. But it also needs to check the return code of 
create_directory(), since it may also be the case that the path exists, but is 
not a directory (a file or symlink is in the way).

Best regards,
-Stephan


Other related posts:

  1. Home
  2. haiku-commits
  3. Archive
  4. 08-2016