Fix (or workaround) quite a few bugs with newusb interaction.

* Remove needless uses of hfp_error which can help avoid assertion crashes.
* Try and avoid a race condition which can lock hal when getting the
  kern.geom.conftxt sysctl.
* Reduce the number of times storage devices are refreshed.
* Fix some bugs with building the newusb device tree.
* Properly detect when newusb devices are added and removed.


git-svn-id: svn://creme-brulee.marcuscom.com/ports/trunk@12874 df743ca5-7f9a-e211-a948-0013205c9059

4 files changed, 107 insertions, 61 deletions

diff --git a/sysutils/hal/Makefile b/sysutils/hal/Makefile
index 18b6ce415..bd656ad44 100644
--- a/sysutils/hal/Makefile
+++ b/sysutils/hal/Makefile
@@ -3,12 +3,12 @@
 # Whom:				Joe Marcus Clarke <marcus@FreeBSD.org>
 #
 # $FreeBSD$
-#   $MCom: ports/sysutils/hal/Makefile,v 1.61 2009/08/19 04:40:26 marcus Exp $
+#   $MCom: ports/sysutils/hal/Makefile,v 1.62 2009/08/19 20:57:23 marcus Exp $
 #
 
 PORTNAME=	hal
 DISTVERSION=	0.5.13
-PORTREVISION=	3
+PORTREVISION=	4
 CATEGORIES=	sysutils
 MASTER_SITES=	http://hal.freedesktop.org/releases/
 
diff --git a/sysutils/hal/files/patch-hald_freebsd_addons_addon-storage.c b/sysutils/hal/files/patch-hald_freebsd_addons_addon-storage.c
index 87b7441a0..f19b7e92d 100644
--- a/sysutils/hal/files/patch-hald_freebsd_addons_addon-storage.c
+++ b/sysutils/hal/files/patch-hald_freebsd_addons_addon-storage.c
@@ -1,5 +1,5 @@
 --- hald/freebsd/addons/addon-storage.c.orig	2008-08-10 09:50:10.000000000 -0400
-+++ hald/freebsd/addons/addon-storage.c	2009-08-08 15:56:20.000000000 -0400
++++ hald/freebsd/addons/addon-storage.c	2009-09-13 17:23:52.000000000 -0400
 @@ -36,17 +36,24 @@
  #include "../libprobe/hfp.h"
  #include "../libprobe/hfp-cdrom.h"
@@ -27,7 +27,17 @@
  /* see MMC-3 Working Draft Revision 10 */
  static boolean
  hf_addon_storage_cdrom_eject_pressed (HFPCDROM *cdrom)
-@@ -144,18 +151,148 @@ hf_addon_storage_update (void)
+@@ -100,8 +107,7 @@ hf_addon_storage_update (void)
+ 
+ 	  if (hf_addon_storage_cdrom_eject_pressed(cdrom))
+ 	    {
+-	      libhal_device_emit_condition(hfp_ctx, hfp_udi, "EjectPressed", "", &hfp_error);
+-	      dbus_error_free(&hfp_error);
++	      libhal_device_emit_condition(hfp_ctx, hfp_udi, "EjectPressed", "", NULL);
+ 	    }
+ 
+ 	  hfp_cdrom_free(cdrom);
+@@ -144,18 +150,142 @@ hf_addon_storage_update (void)
  	}
      }
  
@@ -47,19 +57,17 @@
 +                                                         "block.storage_device",
 +							 hfp_udi,
 +							 &num_volumes,
-+							 &hfp_error)) != NULL)
++							 NULL)) != NULL)
 +    {
 +      int i;
 +
-+      dbus_error_free(&hfp_error);
-+
 +      for (i = 0; i < num_volumes; i++)
 +        {
 +          char *vol_udi;
 +
 +	  vol_udi = volumes[i];
 +
-+	  if (libhal_device_get_property_bool(hfp_ctx, vol_udi, "volume.is_mounted", &hfp_error))
++	  if (libhal_device_get_property_bool(hfp_ctx, vol_udi, "volume.is_mounted", NULL))
 +            {
 +              DBusMessage *msg = NULL;
 +	      DBusMessage *reply = NULL;
@@ -68,7 +76,6 @@
 +	      char **options = NULL;
 +	      char *devfile;
 +
-+	      dbus_error_free(&hfp_error);
 +              hfp_info("Forcing unmount of volume '%s'", vol_udi);
 +
 +	      dbus_connection = libhal_ctx_get_dbus_connection(hfp_ctx);
@@ -149,10 +156,9 @@
 +      check_lock_state = FALSE;
 +
 +      hfp_info("Checking whether device %s is locked by HAL", addon.device_file);
-+      if (libhal_device_is_locked_by_others(hfp_ctx, hfp_udi, "org.freedesktop.Hal.Device.Storage", &hfp_error))
++      if (libhal_device_is_locked_by_others(hfp_ctx, hfp_udi, "org.freedesktop.Hal.Device.Storage", NULL))
 +        {
 +          hfp_info("... device %s is locked by HAL", addon.device_file);
-+	  dbus_error_free(&hfp_error);
 +	  is_locked_by_hal = TRUE;
 +	  update_proc_title(addon.device_file);
 +	  goto skip_check;
@@ -162,10 +168,8 @@
 +          hfp_info("... device %s is not locked by HAL", addon.device_file);
 +	  is_locked_by_hal = FALSE;
 +	}
-+      dbus_error_free(&hfp_error);
 +
-+      should_poll = libhal_device_get_property_bool(hfp_ctx, hfp_udi, "storage.media_check_enabled", &hfp_error);
-+      dbus_error_free(&hfp_error);
++      should_poll = libhal_device_get_property_bool(hfp_ctx, hfp_udi, "storage.media_check_enabled", NULL);
 +      polling_disabled = ! should_poll;
 +      update_proc_title(addon.device_file);
 +    }
@@ -180,7 +184,7 @@
    if (has_media != addon.had_media)
      {
        /*
-@@ -168,6 +305,14 @@ poll_for_media (void)
+@@ -168,27 +298,47 @@ poll_for_media (void)
         * then hung while rebooting and did not unmount my other
         * filesystems.
         */
@@ -193,9 +197,10 @@
 +      unmount_volumes();
 +#endif
  
-       libhal_device_rescan(hfp_ctx, hfp_udi, &hfp_error);
-       dbus_error_free(&hfp_error);
-@@ -175,20 +320,33 @@ poll_for_media (void)
+-      libhal_device_rescan(hfp_ctx, hfp_udi, &hfp_error);
+-      dbus_error_free(&hfp_error);
++      libhal_device_rescan(hfp_ctx, hfp_udi, NULL);
+       addon.had_media = has_media;
  
        return TRUE;
      }
@@ -234,7 +239,7 @@
  {
    if (dbus_message_is_method_call(message,
  			  	  "org.freedesktop.Hal.Device.Storage.Removable",
-@@ -199,7 +357,7 @@ filter_function (DBusConnection *connect
+@@ -199,7 +349,7 @@ filter_function (DBusConnection *connect
  
        hfp_info("Forcing poll for media becusse CheckForMedia() was called");
  
@@ -243,7 +248,7 @@
  
        reply = dbus_message_new_method_return (message);
        dbus_message_append_args(reply, DBUS_TYPE_BOOLEAN, &had_effect, DBUS_TYPE_INVALID);
-@@ -217,8 +375,9 @@ main (int argc, char **argv)
+@@ -217,8 +367,9 @@ main (int argc, char **argv)
    char *removable;
    char *bus;
    char *driver;
@@ -254,19 +259,19 @@
  
    if (! hfp_init(argc, argv))
      goto end;
-@@ -251,16 +410,41 @@ main (int argc, char **argv)
+@@ -251,16 +402,39 @@ main (int argc, char **argv)
    addon.is_scsi_removable = (! strcmp(bus, "scsi") ||
      (! strcmp(bus, "usb") && (! strcmp(driver, "da") || ! strcmp(driver, "sa") ||
      ! strcmp(driver, "cd")))) && ! strcmp(removable, "true");
 -  addon.had_media = hf_addon_storage_update();
 +  addon.had_media = poll_for_media(TRUE, FALSE);
  
-   if (! libhal_device_addon_is_ready(hfp_ctx, hfp_udi, &hfp_error))
+-  if (! libhal_device_addon_is_ready(hfp_ctx, hfp_udi, &hfp_error))
++  if (! libhal_device_addon_is_ready(hfp_ctx, hfp_udi, NULL))
      goto end;
-   dbus_error_free(&hfp_error);
- 
-+  syscon = dbus_bus_get(DBUS_BUS_SYSTEM, &hfp_error);
-+  dbus_error_free(&hfp_error);
+-  dbus_error_free(&hfp_error);
++
++  syscon = dbus_bus_get(DBUS_BUS_SYSTEM, NULL);
 +  assert(syscon != NULL);
 +  dbus_connection_set_exit_on_disconnect(syscon, 0);
 +
@@ -289,7 +294,7 @@
 +  hfp_free(filter_str);
 +
 +  dbus_connection_add_filter(syscon, dbus_filter_function, NULL, NULL);
-+
+ 
    connection = libhal_ctx_get_dbus_connection(hfp_ctx);
    assert(connection != NULL);
    dbus_connection_set_exit_on_disconnect(connection, 0);
@@ -298,7 +303,20 @@
  
    if (! libhal_device_claim_interface(hfp_ctx,
  			 	      hfp_udi,
-@@ -280,40 +464,32 @@ main (int argc, char **argv)
+@@ -268,52 +442,43 @@ main (int argc, char **argv)
+ 				      "    <method name=\"CheckForMedia\">\n"
+ 				      "      <arg name=\"call_had_sideeffect\" direction=\"out\" type=\"b\"/>\n"
+ 				      "    </method>\n",
+-				      &hfp_error))
++				      NULL))
+     {
+       hfp_critical("Cannot claim interface 'org.freedesktop.Hal.Device.Storage.Removable'");
+       goto end;
+     }
+-  dbus_error_free(&hfp_error);
+ 
+   while (TRUE)
+     {
        /* process dbus traffic until update interval has elapsed */
        while (TRUE)
  	{
diff --git a/sysutils/hal/files/patch-hald_freebsd_hf-usb2.c b/sysutils/hal/files/patch-hald_freebsd_hf-usb2.c
index cd78786c0..3afc57966 100644
--- a/sysutils/hal/files/patch-hald_freebsd_hf-usb2.c
+++ b/sysutils/hal/files/patch-hald_freebsd_hf-usb2.c
@@ -1,8 +1,8 @@
---- hald/freebsd/hf-usb2.c.orig	2009-03-02 20:16:10.000000000 -0600
-+++ hald/freebsd/hf-usb2.c	2009-03-02 20:33:13.000000000 -0600
-@@ -0,0 +1,287 @@
+--- hald/freebsd/hf-usb2.c.orig	2009-09-12 21:16:02.000000000 -0400
++++ hald/freebsd/hf-usb2.c	2009-09-13 20:57:00.000000000 -0400
+@@ -0,0 +1,312 @@
 +/***************************************************************************
-+ * CVSID: $Id: patch-hald_freebsd_hf-usb2.c,v 1.1 2009-05-17 05:02:48 marcus Exp $
++ * CVSID: $Id: patch-hald_freebsd_hf-usb2.c,v 1.2 2009-09-14 04:34:35 marcus Exp $
 + *
 + * hf-usb.c : USB support
 + *
@@ -199,7 +199,8 @@
 +      else
 +        parent = hf_device_store_match(hald_get_gdl(), "usb_device.bus_number",
 +          HAL_PROPERTY_TYPE_INT32, bus, "usb_device.port_number",
-+	  HAL_PROPERTY_TYPE_INT32, addr - 1, NULL);
++	  HAL_PROPERTY_TYPE_INT32, addr - 1, "info.bus",
++	  HAL_PROPERTY_TYPE_STRING, "usb_device", NULL);
 +      if (! parent || hal_device_property_get_bool(parent, "info.ignore"))
 +        continue;
 +
@@ -219,7 +220,13 @@
 +  HalDevice *parent_device;
 +  int bus, addr, pbus, paddr;
 +
-+  if (strncmp(name, "ugen", strlen("ugen")))
++  if (! parent)
++    return FALSE;
++
++  if (strncmp(name, "ugen", strlen("ugen")) &&
++      ! strncmp(parent, "uhub", strlen("uhub")))
++    return TRUE;
++  else if (strncmp(name, "ugen", strlen("ugen")))
 +    return FALSE;
 +  else if (strncmp(parent, "ugen", strlen("ugen")))
 +    return TRUE;
@@ -235,7 +242,8 @@
 +
 +  parent_device = hf_device_store_match(hald_get_gdl(),
 +    "usb_device.bus_number", HAL_PROPERTY_TYPE_INT32, pbus,
-+    "usb_device.port_number", HAL_PROPERTY_TYPE_INT32, paddr, NULL);
++    "usb_device.port_number", HAL_PROPERTY_TYPE_INT32, paddr, "info.bus",
++    HAL_PROPERTY_TYPE_STRING, "usb_device", NULL);
 +
 +  if (parent_device && ! hal_device_property_get_bool(parent_device,
 +      "info.ignore"))
@@ -258,8 +266,6 @@
 +
 +  if (strncmp(name, "ugen", strlen("ugen")))
 +    return FALSE;
-+  else if (strncmp(parent, "ugen", strlen("ugen")))
-+    return TRUE;
 +
 +  if (sscanf(name, "ugen%i.%i", &bus, &addr) != 2)
 +    return FALSE;
@@ -268,7 +274,8 @@
 +
 +  device = hf_device_store_match(hald_get_gdl(), "usb_device.bus_number",
 +    HAL_PROPERTY_TYPE_INT32, bus, "usb_device.port_number",
-+    HAL_PROPERTY_TYPE_INT32, addr, NULL);
++    HAL_PROPERTY_TYPE_INT32, addr, "info.bus",
++    HAL_PROPERTY_TYPE_STRING, "usb_device", NULL);
 +
 +  if (device)
 +    {
@@ -279,6 +286,23 @@
 +  return FALSE;
 +}
 +
++static gboolean
++hf_usb2_devd_notify (const char *system,
++		     const char *subsystem,
++		     const char *type,
++		     const char *data)
++{
++  if (! data || strcmp(system, "DEVFS") || strcmp(subsystem, "CDEV") ||
++      (strcmp(type, "CREATE") && strcmp(type, "DESTROY")))
++    return FALSE;
++
++  if (! strncmp(data, "cdev=ugen", strlen("cdev=ugen")) ||
++      ! strncmp(data, "cdev=usb", strlen("cdev=usb")))
++    return TRUE;
++
++  return FALSE;
++}
++
 +HFHandler hf_usb2_handler = {
 +  .privileged_init	= hf_usb2_privileged_init,
 +  .probe		= hf_usb2_probe
@@ -286,5 +310,6 @@
 +
 +HFDevdHandler hf_usb2_devd_handler = {
 +  .add =	hf_usb2_devd_add,
-+  .remove =	hf_usb2_devd_remove
++  .remove =	hf_usb2_devd_remove,
++  .notify =     hf_usb2_devd_notify
 +};
diff --git a/sysutils/hal/files/patch-hald_hf-storage.c b/sysutils/hal/files/patch-hald_hf-storage.c
index 605342c36..40cccef79 100644
--- a/sysutils/hal/files/patch-hald_hf-storage.c
+++ b/sysutils/hal/files/patch-hald_hf-storage.c
@@ -1,14 +1,15 @@
---- hald/freebsd/hf-storage.c.orig	2008-08-10 09:50:10.000000000 -0400
-+++ hald/freebsd/hf-storage.c	2009-05-23 18:52:52.000000000 -0400
-@@ -30,6 +30,7 @@
+--- hald/freebsd/hf-storage.c.orig	2009-05-12 08:24:28.000000000 -0400
++++ hald/freebsd/hf-storage.c	2009-09-13 20:48:38.000000000 -0400
+@@ -30,6 +30,8 @@
  #include <limits.h>
  #include <inttypes.h>
  #include <string.h>
++#include <unistd.h>
 +#include <sys/param.h>
  #include <sys/types.h>
  #include <sys/disklabel.h>
  
-@@ -38,6 +39,7 @@
+@@ -38,6 +40,7 @@
  
  #include "hf-storage.h"
  #include "hf-block.h"
@@ -16,7 +17,7 @@
  #include "hf-devtree.h"
  #include "hf-volume.h"
  #include "hf-util.h"
-@@ -64,7 +66,7 @@ typedef struct
+@@ -64,7 +67,7 @@ typedef struct
  static GNode *hf_storage_geom_tree = NULL;
  static GHashTable *hf_storage_geom_hash = NULL;
  
@@ -25,7 +26,7 @@
  static gboolean hf_storage_device_has_addon (HalDevice *device);
  
  static void
-@@ -104,6 +106,8 @@ hf_storage_class_is_partitionable (const
+@@ -104,6 +107,8 @@ hf_storage_class_is_partitionable (const
  {
    return (! strcmp(geom_class, "MBR") ||
            ! strcmp(geom_class, "MBREXT") ||
@@ -34,7 +35,7 @@
  	  ! strcmp(geom_class, "GPT") ||
            ! strcmp(geom_class, "APPLE") || ! strcmp(geom_class, "SUN"));
  }
-@@ -117,6 +121,7 @@ hf_storage_geom_has_partitions (const Ge
+@@ -117,6 +122,7 @@ hf_storage_geom_has_partitions (const Ge
    if (g_node_n_children(node) > 0)
      return TRUE;
  
@@ -42,7 +43,7 @@
    if (hf_storage_class_is_partitionable(geom_obj->class) &&
        g_node_next_sibling(node) != NULL)
      {
-@@ -135,6 +140,7 @@ hf_storage_geom_has_partitions (const Ge
+@@ -135,6 +141,7 @@ hf_storage_geom_has_partitions (const Ge
              return TRUE;
          }
      }
@@ -50,7 +51,7 @@
  
    return FALSE;
  }
-@@ -294,7 +300,7 @@ hf_storage_device_probe (HalDevice *devi
+@@ -294,7 +301,7 @@ hf_storage_device_probe (HalDevice *devi
  {
    g_return_if_fail(HAL_IS_DEVICE(device));
  
@@ -59,7 +60,7 @@
  
    if (hf_runner_run_sync(device, 0, "hald-probe-storage",
  			 "HF_HAS_CHILDREN", HF_BOOL_TO_STRING(hf_storage_device_has_partitions(device)),
-@@ -403,13 +409,20 @@ hf_storage_parse_conftxt (const char *co
+@@ -403,13 +410,20 @@ hf_storage_parse_conftxt (const char *co
            continue;
  	}
  
@@ -82,7 +83,7 @@
        geom_obj->hash = hash;
  
        if (g_strv_length(fields) >= 5)
-@@ -433,6 +446,30 @@ hf_storage_parse_conftxt (const char *co
+@@ -433,6 +447,30 @@ hf_storage_parse_conftxt (const char *co
                if (! strcmp (geom_obj->class, "GPT") ||
                    ! strcmp (geom_obj->class, "APPLE"))
                  geom_obj->str_type = g_strdup(fields[10]);
@@ -113,7 +114,7 @@
  	      else
                  geom_obj->type = atoi(fields[10]);
  	    }
-@@ -541,15 +578,20 @@ hf_storage_device_rescan_real (HalDevice
+@@ -541,15 +579,22 @@ hf_storage_device_rescan_real (HalDevice
  }
  
  static gboolean
@@ -131,13 +132,15 @@
  
 -  if (hf_is_waiting)
 -    return TRUE;
-+  if (strcmp(system, "DEVFS") || strcmp(subsystem, "CDEV") ||
++  if (! data || strcmp(system, "DEVFS") || strcmp(subsystem, "CDEV") ||
 +      (strcmp(type, "CREATE") && strcmp(type, "DESTROY")))
 +    return FALSE;
++
++  sleep(1);
  
    conftxt = hf_get_string_sysctl(NULL, "kern.geom.conftxt");
    new_disks = hf_storage_parse_conftxt(conftxt);
-@@ -572,6 +614,7 @@ hf_storage_conftxt_timeout_cb (gpointer 
+@@ -572,6 +617,7 @@ hf_storage_conftxt_timeout_cb (gpointer 
  	  if (! hf_storage_find_disk(disks, disk->name))
  	    {
  	      osspec_probe();	/* catch new disk(s) */
@@ -145,7 +148,7 @@
  	      break;
  	    }
  	}
-@@ -593,7 +636,10 @@ hf_storage_conftxt_timeout_cb (gpointer 
+@@ -593,7 +639,10 @@ hf_storage_conftxt_timeout_cb (gpointer 
  		  device = hf_devtree_find_from_name(hald_get_gdl(), disk->name);
  		  if (device && hal_device_has_capability(device, "storage") &&
  		      ! hf_storage_device_has_addon(device))
@@ -157,7 +160,7 @@
  		}
  	    }
  	  else
-@@ -601,7 +647,10 @@ hf_storage_conftxt_timeout_cb (gpointer 
+@@ -601,7 +650,10 @@ hf_storage_conftxt_timeout_cb (gpointer 
  	      /* disk removed */
  	      device = hf_devtree_find_from_name(hald_get_gdl(), disk->name);
  	      if (device && hal_device_has_capability(device, "storage"))
@@ -169,7 +172,7 @@
  	    }
  	}
      }
-@@ -610,17 +659,30 @@ hf_storage_conftxt_timeout_cb (gpointer 
+@@ -610,17 +662,30 @@ hf_storage_conftxt_timeout_cb (gpointer 
    g_slist_free(disks);
    disks = new_disks;
  
@@ -183,7 +186,7 @@
 +  if (hf_is_waiting)
 +    return TRUE;
 +
-+  hf_storage_devd_notify("DEVFS", "CDEV", "CREATE", NULL);
++  hf_storage_devd_notify("DEVFS", "CDEV", "CREATE", "");
 +
    return TRUE;
  }
@@ -202,7 +205,7 @@
      return;
  
    conftxt = hf_get_string_sysctl(NULL, "kern.geom.conftxt");
-@@ -636,8 +698,10 @@ hf_storage_init_geom (void)
+@@ -636,8 +701,10 @@ hf_storage_init_geom (void)
  static void
  hf_storage_init (void)
  {
@@ -214,7 +217,7 @@
  }
  
  void
-@@ -720,8 +784,6 @@ hf_storage_device_add (HalDevice *device
+@@ -719,8 +786,6 @@ hf_storage_device_add (HalDevice *device
  {
    g_return_if_fail(HAL_IS_DEVICE(device));
  
@@ -223,7 +226,7 @@
    if (hf_device_preprobe(device))
      {
        hf_storage_device_probe(device, FALSE);
-@@ -739,7 +801,7 @@ hf_storage_get_geoms (const char *devnam
+@@ -738,7 +803,7 @@ hf_storage_get_geoms (const char *devnam
  
    g_return_val_if_fail(devname != NULL, NULL);
  
@@ -232,7 +235,7 @@
  
    hash = g_str_hash(devname);
    node = g_node_find(hf_storage_geom_tree, G_PRE_ORDER, G_TRAVERSE_ALL,
-@@ -802,3 +864,7 @@ HFHandler hf_storage_handler = {
+@@ -801,3 +866,7 @@ HFHandler hf_storage_handler = {
    .probe =		hf_storage_probe,
    .device_rescan =	hf_storage_device_rescan
  };