Re: [gpm] hal.systems.formfactor

[Danny Kukawka <danny kukawka web de>]

On Monday 24 April 2006 00:41, Richard Hughes wrote:
> I've talked with Fabian before, on and off list about
> set-low-power-mode. I'll think more about a more complete fix (as I have
> data from previous emails), but this fixes the issue for him, for now.

Here is a fix for this problem [patch is again CVS-HEAD, if you need a patch 
e.g. against 0.5.6/7 let me know ... I didn't read the archive for the HAL 
version, you also could have simply written which version it is ;-)]. Please 
check if this help.

> When I post something to hal freedesktop, you can treat me like an
> idiot. 

Okay, I take you at your word. ;-)


Cheers,

Danny

----------  Forwarded Message  ----------

Subject: [PATCH] fix mapping of chassitype to system.formfactor
Date: Friday 05 May 2006 17:48
From: Danny Kukawka <danny kukawka web de>
To: hal lists freedesktop org

Hi,

this patch fix the mapping of smbios.chassis.type to system.formfactor. I
added a device_property_atomic_update_begin()/end() around the mapping and a
check if the property is already set to avoid overwrite values e.g. set in
the ACPI/APM/PMU code. With this patch I changed/moved set a default value
('unkown') for system.formfactor.

Also added a check for the value of exit_type to skip mapping if execution of
the smbios prober failed.

Please review.

Cheers,

Danny

 ChangeLog            |   11 +++
 hald/linux2/osspec.c |  129 ++++++++++++++++++++++-------------------
 2 files changed, 81 insertions(+), 59 deletions(-)

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

Index: ChangeLog
===================================================================
RCS file: /cvs/hal/hal/ChangeLog,v
retrieving revision 1.882
diff -u -3 -p -u -r1.882 ChangeLog
--- ChangeLog	4 May 2006 19:40:16 -0000	1.882
+++ ChangeLog	5 May 2006 10:56:07 -0000
@@ -1,3 +1,14 @@
+2006-05-05  Danny Kukawka  <danny kukawka web de>
+
+	* hald/linux2/osspec.c: (computer_probing_pcbios_helper_done): Fixed 
+	mapping of system.formfactor from smbios.chassis.type. Added 
+	atomic_update around the mapping and a check if the key is already
+	set to avoid overwrite values e.g. set in the ACPI/APM/PMU code. 
+	Added check for exit_type to skip mapping if execution of the smbios
+	prober failed.
+	(osspec_probe): Removed set default value for system.formfactor for
+	all cases and readded to case if !should_decode_dmi is set.
+
 2006-05-04  David Zeuthen  <davidz redhat com>
 
 	Patch from Joe Marcus Clarke <marcus freebsd org>. Move sockets to
Index: hald/linux2/osspec.c
===================================================================
RCS file: /cvs/hal/hal/hald/linux2/osspec.c,v
retrieving revision 1.46
diff -u -3 -p -u -r1.46 osspec.c
--- hald/linux2/osspec.c	1 May 2006 21:32:54 -0000	1.46
+++ hald/linux2/osspec.c	5 May 2006 10:56:07 -0000
@@ -360,14 +360,22 @@ computer_probing_helper_done (HalDevice 
 
 static void 
 computer_probing_pcbios_helper_done (HalDevice *d, guint32 exit_type, 
-		                                 gint return_code, gchar **error, 
-				                             gpointer data1, gpointer data2)
+	                             gint return_code, gchar **error, 
+				     gpointer data1, gpointer data2)
 {
 	const char *chassis_type;
 	const char *system_manufacturer;
 	const char *system_product;
 	const char *system_version;
 
+	if (exit_type == HALD_RUN_FAILED) {
+		/* set a default value */
+		device_property_atomic_update_begin ();
+		hal_device_property_set_string (d, "system.formfactor", "unknown");
+		device_property_atomic_update_end ();
+		goto out;
+	}
+
 	if ((system_manufacturer = hal_device_property_get_string (d, "smbios.system.manufacturer")) != NULL &&
 	    (system_product = hal_device_property_get_string (d, "smbios.system.product")) != NULL &&
 	    (system_version = hal_device_property_get_string (d, "smbios.system.version")) != NULL) {
@@ -384,57 +392,65 @@ computer_probing_pcbios_helper_done (Hal
 	}
 
 
-	/* now map the smbios.* properties to our generic system.formfactor property */
-	if ((chassis_type = hal_device_property_get_string (d, "smbios.chassis.type")) != NULL) {
-		unsigned int i;
-
-		/* Map the chassis type from dmidecode.c to a sensible type used in hal 
-		 *
-		 * See also 3.3.4.1 of the "System Management BIOS Reference Specification, 
-		 * Version 2.3.4" document, available from http://www.dmtf.org/standards/smbios.
-		 *
-		 * TODO: figure out WTF the mapping should be; "Lunch Box"? Give me a break :-)
-		 */
-		static const char *chassis_map[] = {
-			"Other",                 "unknown",
-			"Unknown",               "unknown",
-			"Desktop",               "desktop",
-			"Low Profile Desktop",   "desktop",
-			"Pizza Box",             "server",
-			"Mini Tower",            "desktop",
-			"Tower",                 "desktop",
-			"Portable",              "laptop",
-			"Laptop",                "laptop",
-			"Notebook",              "laptop",
-			"Hand Held",             "handheld",
-			"Docking Station",       "laptop",
-			"All In One",            "unknown",
-			"Sub Notebook",          "laptop",
-			"Space-saving",          "unknown",
-			"Lunch Box",             "unknown",
-			"Main Server Chassis",   "server",
-			"Expansion Chassis",     "unknown",
-			"Sub Chassis",           "unknown",
-			"Bus Expansion Chassis", "unknown",
-			"Peripheral Chassis",    "unknown",
-			"RAID Chassis",          "unknown",
-			"Rack Mount Chassis",    "unknown",
-			"Sealed-case PC",        "unknown",
-			"Multi-system",          "unknown",
-			NULL
-		};
-
-		for (i = 0; chassis_map[i] != NULL; i += 2) {
-			if (strcmp (chassis_map[i], chassis_type) == 0) {
-				/* check if the key is 'unknown' to prevent overwrite keys */
-				if (strcmp (hal_device_property_get_string (d, "system.formfactor"), "unknown"))
+	/* Use a atomic lock to avoid trouble/overwrite already set value for system.formfactor
+	 * from other code paths (e.g. in ACPI/APM/PMU code) 
+	 */
+	device_property_atomic_update_begin ();
+	if (!hal_device_has_property (d, "system.formfactor")) {
+		/* now map the smbios.* properties to our generic system.formfactor property */
+		if ((chassis_type = hal_device_property_get_string (d, "smbios.chassis.type")) != NULL) {
+			unsigned int i;
+
+			/* Map the chassis type from dmidecode.c to a sensible type used in hal 
+			 *
+			 * See also 3.3.4.1 of the "System Management BIOS Reference Specification, 
+			 * Version 2.3.4" document, available from http://www.dmtf.org/standards/smbios.
+			 *
+			 * TODO: figure out WTF the mapping should be; "Lunch Box"? Give me a break :-)
+			 */
+			static const char *chassis_map[] = {
+				"Other",                 "unknown",
+				"Unknown",               "unknown",
+				"Desktop",               "desktop",
+				"Low Profile Desktop",   "desktop",
+				"Pizza Box",             "server",
+				"Mini Tower",            "desktop",
+				"Tower",                 "desktop",
+				"Portable",              "laptop",
+				"Laptop",                "laptop",
+				"Notebook",              "laptop",
+				"Hand Held",             "handheld",
+				"Docking Station",       "laptop",
+				"All In One",            "unknown",
+				"Sub Notebook",          "laptop",
+				"Space-saving",          "unknown",
+				"Lunch Box",             "unknown",
+				"Main Server Chassis",   "server",
+				"Expansion Chassis",     "unknown",
+				"Sub Chassis",           "unknown",
+				"Bus Expansion Chassis", "unknown",
+				"Peripheral Chassis",    "unknown",
+				"RAID Chassis",          "unknown",
+				"Rack Mount Chassis",    "unknown",
+				"Sealed-case PC",        "unknown",
+				"Multi-system",          "unknown",
+				NULL
+			};
+
+			for (i = 0; chassis_map[i] != NULL; i += 2) {
+				if (strcmp (chassis_map[i], chassis_type) == 0) {
 					hal_device_property_set_string (d, "system.formfactor", chassis_map[i+1]);
-				break;
+					break;
+				}
 			}
+		       
+		} else {
+			/* set a default value */
+			hal_device_property_set_string (d, "system.formfactor", "unknown");
 		}
-	       
 	}
-
+	device_property_atomic_update_end ();
+out:
 	computer_probing_helper_done (d);
 }
 
@@ -503,10 +519,6 @@ osspec_probe (void)
 		hal_device_property_set_string (root, "system.kernel.machine", un.machine);
 	}
 
-	/* can be overridden by dmidecode, others */
-	hal_device_property_set_string (root, "system.formfactor", "unknown");
-
-
 	/* Let computer be in TDL while synthesizing all other events because some may write to the object */
 	hal_device_store_add (hald_get_tdl (), root);
 
@@ -538,15 +550,14 @@ osspec_probe (void)
 
 	/* TODO: add prober for PowerMac's */
 	if (should_decode_dmi) {
-		hald_runner_run (root, "hald-probe-smbios", NULL,
-                         HAL_HELPER_TIMEOUT,
-                         computer_probing_pcbios_helper_done, 
-                         NULL, NULL);
+		hald_runner_run (root, "hald-probe-smbios", NULL, HAL_HELPER_TIMEOUT,
+        	                 computer_probing_pcbios_helper_done, NULL, NULL);
 	} else {
+		/* set a default value, can be overridden by others */
+		hal_device_property_set_string (root, "system.formfactor", "unknown");
 		/* no probing */
 		computer_probing_helper_done (root);
-  }
-
+  	}
 }
 
 DBusHandlerResult