[settings] Eliminate settings "tag magic"

Create an explicit concept of "settings scope" and eliminate the magic
values used for numerical setting tags.

Signed-off-by: Michael Brown <mcb30@ipxe.org>

src/arch/i386/interface/vmware/guestinfo.c

@@ -224,7 +224,7 @@ static int guestinfo_net_probe ( struct net_device *netdev ) {
 		rc = -ENOMEM;
 		goto err_alloc;
 	}
-	settings_init ( settings, &guestinfo_settings_operations, NULL, 0 );
+	settings_init ( settings, &guestinfo_settings_operations, NULL, NULL );
 
 	/* Register settings */
 	if ( ( rc = register_settings ( settings, netdev_settings ( netdev ),

src/core/nvo.c

@@ -195,7 +195,8 @@ static int nvo_save ( struct nvo_block *nvo ) {
 int nvo_applies ( struct settings *settings __unused,
 		  struct setting *setting ) {
 
-	return dhcpopt_applies ( setting->tag );
+	return ( ( setting->scope == NULL ) &&
+		 dhcpopt_applies ( setting->tag ) );
 }
 
 /**
@@ -274,7 +275,8 @@ void nvo_init ( struct nvo_block *nvo, struct nvs_device *nvs,
 	nvo->len = len;
 	nvo->resize = resize;
 	dhcpopt_init ( &nvo->dhcpopts, NULL, 0, nvo_realloc_dhcpopt );
-	settings_init ( &nvo->settings, &nvo_settings_operations, refcnt, 0 );
+	settings_init ( &nvo->settings, &nvo_settings_operations,
+			refcnt, NULL );
 }
 
 /**

src/core/settings.c

@@ -984,7 +984,7 @@ void clear_settings ( struct settings *settings ) {
 int setting_cmp ( struct setting *a, struct setting *b ) {
 
 	/* If the settings have tags, compare them */
-	if ( a->tag && ( a->tag == b->tag ) )
+	if ( a->tag && ( a->tag == b->tag ) && ( a->scope == b->scope ) )
 		return 0;
 
 	/* Otherwise, if the settings have names, compare them */
@@ -1099,19 +1099,17 @@ struct setting * find_setting ( const char *name ) {
 /**
  * Parse setting name as tag number
  *
- * @v settings		Settings block
  * @v name		Name
  * @ret tag		Tag number, or 0 if not a valid number
  */
-static unsigned int parse_setting_tag ( struct settings *settings,
-					const char *name ) {
+static unsigned int parse_setting_tag ( const char *name ) {
 	char *tmp = ( ( char * ) name );
 	unsigned int tag = 0;
 
 	while ( 1 ) {
 		tag = ( ( tag << 8 ) | strtoul ( tmp, &tmp, 0 ) );
 		if ( *tmp == 0 )
-			return ( tag | settings->tag_magic );
+			return tag;
 		if ( *tmp != '.' )
 			return 0;
 		tmp++;
@@ -1193,7 +1191,8 @@ parse_setting_name ( const char *name,
 	}
 
 	/* Identify setting */
-	setting->tag = parse_setting_tag ( *settings, setting_name );
+	setting->tag = parse_setting_tag ( setting_name );
+	setting->scope = (*settings)->default_scope;
 	setting->name = setting_name;
 	for_each_table_entry ( named_setting, SETTINGS ) {
 		/* Matches a defined named setting; use that setting */
@@ -1998,26 +1997,15 @@ struct builtin_setting_operation {
 	int ( * fetch ) ( void *data, size_t len );
 };
 
-/** Built-in setting tag magic */
-#define BUILTIN_SETTING_TAG_MAGIC 0xb1
-
-/**
- * Construct built-in setting tag
- *
- * @v id		Unique identifier
- * @ret tag		Setting tag
- */
-#define BUILTIN_SETTING_TAG( id ) ( ( BUILTIN_SETTING_TAG_MAGIC << 24 ) | (id) )
-
-/** "errno" setting tag */
-#define BUILTIN_SETTING_TAG_ERRNO BUILTIN_SETTING_TAG ( 0x01 )
+/** Built-in setting scope */
+static struct settings_scope builtin_scope;
 
 /** Error number setting */
 struct setting errno_setting __setting ( SETTING_MISC ) = {
 	.name = "errno",
 	.description = "Last error",
-	.tag = BUILTIN_SETTING_TAG_ERRNO,
 	.type = &setting_type_uint32,
+	.scope = &builtin_scope,
 };
 
 /**
@@ -2038,15 +2026,12 @@ static int errno_fetch ( void *data, size_t len ) {
 	return sizeof ( content );
 }
 
-/** "buildarch" setting tag */
-#define BUILTIN_SETTING_TAG_BUILDARCH BUILTIN_SETTING_TAG ( 0x02 )
-
 /** Build architecture setting */
 struct setting buildarch_setting __setting ( SETTING_MISC ) = {
 	.name = "buildarch",
 	.description = "Build architecture",
-	.tag = BUILTIN_SETTING_TAG_BUILDARCH,
 	.type = &setting_type_string,
+	.scope = &builtin_scope,
 };
 
 /**
@@ -2063,15 +2048,12 @@ static int buildarch_fetch ( void *data, size_t len ) {
 	return ( sizeof ( buildarch ) - 1 /* NUL */ );
 }
 
-/** "platform" setting tag */
-#define BUILTIN_SETTING_TAG_PLATFORM BUILTIN_SETTING_TAG ( 0x03 )
-
 /** Platform setting */
 struct setting platform_setting __setting ( SETTING_MISC ) = {
 	.name = "platform",
 	.description = "Platform",
-	.tag = BUILTIN_SETTING_TAG_PLATFORM,
 	.type = &setting_type_string,
+	.scope = &builtin_scope,
 };
 
 /**
@@ -2128,11 +2110,8 @@ static int builtin_fetch ( struct settings *settings __unused,
  */
 static int builtin_applies ( struct settings *settings __unused,
 			     struct setting *setting ) {
-	unsigned int tag_magic;
 
-	/* Check tag magic */
-	tag_magic = ( setting->tag >> 24 );
-	return ( tag_magic == BUILTIN_SETTING_TAG_MAGIC );
+	return ( setting->scope == &builtin_scope );
 }
 
 /** Built-in settings operations */
@@ -2144,7 +2123,6 @@ static struct settings_operations builtin_settings_operations = {
 /** Built-in settings */
 static struct settings builtin_settings = {
 	.refcnt = NULL,
-	.tag_magic = BUILTIN_SETTING_TAG ( 0 ),
 	.siblings = LIST_HEAD_INIT ( builtin_settings.siblings ),
 	.children = LIST_HEAD_INIT ( builtin_settings.children ),
 	.op = &builtin_settings_operations,

src/drivers/net/phantom/phantom.c

@@ -1454,11 +1454,8 @@ static struct net_device_operations phantom_operations = {
  *
  */
 
-/** Phantom CLP settings tag magic */
-#define PHN_CLP_TAG_MAGIC 0xc19c1900UL
-
-/** Phantom CLP settings tag magic mask */
-#define PHN_CLP_TAG_MAGIC_MASK 0xffffff00UL
+/** Phantom CLP settings scope */
+static struct settings_scope phantom_settings_scope;
 
 /** Phantom CLP data
  *
@@ -1689,8 +1686,8 @@ phantom_clp_setting ( struct phantom_nic *phantom, struct setting *setting ) {
 	}
 
 	/* Allow for use of numbered settings */
-	if ( ( setting->tag & PHN_CLP_TAG_MAGIC_MASK ) == PHN_CLP_TAG_MAGIC )
-		return ( setting->tag & ~PHN_CLP_TAG_MAGIC_MASK );
+	if ( setting->scope == &phantom_settings_scope )
+		return setting->tag;
 
 	DBGC2 ( phantom, "Phantom %p has no \"%s\" setting\n",
 		phantom, setting->name );
@@ -2075,7 +2072,7 @@ static int phantom_probe ( struct pci_device *pci ) {
 	assert ( phantom->port < PHN_MAX_NUM_PORTS );
 	settings_init ( &phantom->settings,
 			&phantom_settings_operations,
-			&netdev->refcnt, PHN_CLP_TAG_MAGIC );
+			&netdev->refcnt, &phantom_settings_scope );
 
 	/* Fix up PCI device */
 	adjust_pci_device ( pci );

src/include/ipxe/net80211.h

@@ -1183,25 +1183,4 @@ static inline u16 net80211_cts_duration ( struct net80211_device *dev,
 		 net80211_duration ( dev, size, dev->rates[dev->rate] ) );
 }
 
-/** 802.11 device setting tag magic */
-#define NET80211_SETTING_TAG_MAGIC 0x8211
-
-/**
- * Construct 802.11 setting tag
- *
- * @v id		Unique identifier
- * @ret tag		Setting tag
- */
-#define NET80211_SETTING_TAG( id ) \
-	NETDEV_SETTING_TAG ( ( NET80211_SETTING_TAG_MAGIC << 8 ) | (id) )
-
-/** SSID setting tag */
-#define NET80211_SETTING_TAG_SSID NET80211_SETTING_TAG ( 0x01 )
-
-/** Active scanning setting tag */
-#define NET80211_SETTING_TAG_ACTIVE_SCAN NET80211_SETTING_TAG ( 0x02 )
-
-/** Wireless key setting tag */
-#define NET80211_SETTING_TAG_KEY NET80211_SETTING_TAG ( 0x03 )
-
 #endif

src/include/ipxe/netdevice.h

@@ -411,38 +411,6 @@ struct net_driver {
 /** Declare a network driver */
 #define __net_driver __table_entry ( NET_DRIVERS, 01 )
 
-/** Network device setting tag magic
- *
- * All DHCP option settings are deemed to be valid as network device
- * settings.  There are also some extra non-DHCP settings (such as
- * "mac"), which are marked as being valid network device settings by
- * using a magic tag value.
- */
-#define NETDEV_SETTING_TAG_MAGIC 0xeb
-
-/**
- * Construct network device setting tag
- *
- * @v id		Unique identifier
- * @ret tag		Setting tag
- */
-#define NETDEV_SETTING_TAG( id ) ( ( NETDEV_SETTING_TAG_MAGIC << 24 ) | (id) )
-
-/**
- * Check if tag is a network device setting tag
- *
- * @v tag		Setting tag
- * @ret is_ours		Tag is a network device setting tag
- */
-#define IS_NETDEV_SETTING_TAG( tag ) \
-	( ( (tag) >> 24 ) == NETDEV_SETTING_TAG_MAGIC )
-
-/** MAC address setting tag */
-#define NETDEV_SETTING_TAG_MAC NETDEV_SETTING_TAG ( 0x01 )
-
-/** Bus ID setting tag */
-#define NETDEV_SETTING_TAG_BUS_ID NETDEV_SETTING_TAG ( 0x02 )
-
 extern struct list_head net_devices;
 extern struct net_device_operations null_netdev_operations;
 extern struct settings_operations netdev_settings_operations;

src/include/ipxe/settings.h

@@ -38,27 +38,14 @@ struct setting {
 	 * The setting tag is a numerical description of the setting
 	 * (such as a DHCP option number, or an SMBIOS structure and
 	 * field number).
-	 *
-	 * Users can construct tags for settings that are not
-	 * explicitly known to iPXE using the generic syntax for
-	 * numerical settings.  For example, the setting name "60"
-	 * will be interpreted as referring to DHCP option 60 (the
-	 * vendor class identifier).
-	 *
-	 * This creates a potential for namespace collisions, since
-	 * the interpretation of the numerical description will vary
-	 * according to the settings block.  When a user attempts to
-	 * fetch a generic numerical setting, we need to ensure that
-	 * only the intended settings block interprets the numerical
-	 * description.  (For example, we do not want to attempt to
-	 * retrieve the subnet mask from SMBIOS, or the system UUID
-	 * from DHCP.)
-	 *
-	 * This potential problem is resolved by allowing the setting
-	 * tag to include a "magic" value indicating the
-	 * interpretation to be placed upon the numerical description.
 	 */
 	unsigned int tag;
+	/** Setting scope (or NULL)
+	 *
+	 * For historic reasons, a NULL scope with a non-zero tag
+	 * indicates a DHCPv4 option setting.
+	 */
+	struct settings_scope *scope;
 };
 
 /** Configuration setting table */
@@ -134,12 +121,6 @@ struct settings {
 	struct refcnt *refcnt;
 	/** Name */
 	const char *name;
-	/** Tag magic
-	 *
-	 * This value will be ORed in to any numerical tags
-	 * constructed by parse_setting_name().
-	 */
-	unsigned int tag_magic;
 	/** Parent settings block */
 	struct settings *parent;
 	/** Sibling settings blocks */
@@ -148,8 +129,44 @@ struct settings {
 	struct list_head children;
 	/** Settings block operations */
 	struct settings_operations *op;
+	/** Default scope for numerical settings constructed for this block */
+	struct settings_scope *default_scope;
 };
 
+/**
+ * A setting scope
+ *
+ * Users can construct tags for settings that are not explicitly known
+ * to iPXE using the generic syntax for numerical settings.  For
+ * example, the setting name "60" will be interpreted as referring to
+ * DHCP option 60 (the vendor class identifier).
+ *
+ * This creates a potential for namespace collisions, since the
+ * interpretation of the numerical description will vary according to
+ * the settings block.  When a user attempts to fetch a generic
+ * numerical setting, we need to ensure that only the intended
+ * settings blocks interpret this numerical description.  (For
+ * example, we do not want to attempt to retrieve the subnet mask from
+ * SMBIOS, or the system UUID from DHCP.)
+ *
+ * This potential problem is resolved by including a user-invisible
+ * "scope" within the definition of each setting.  Settings blocks may
+ * use this to determine whether or not the setting is applicable.
+ * Any settings constructed from a numerical description
+ * (e.g. "smbios/1.4.0") will be assigned the default scope of the
+ * settings block specified in the description (e.g. "smbios"); this
+ * provides behaviour matching the user's expectations in most
+ * circumstances.
+ */
+struct settings_scope {
+	/** Dummy field
+	 *
+	 * This is included only to ensure that pointers to different
+	 * scopes always compare differently.
+	 */
+	uint8_t dummy;
+} __attribute__ (( packed ));
+
 /**
  * A setting type
  *
@@ -329,17 +346,17 @@ extern struct setting busid_setting __setting ( SETTING_NETDEV );
  * @v settings		Settings block
  * @v op		Settings block operations
  * @v refcnt		Containing object reference counter, or NULL
- * @v tag_magic		Tag magic
+ * @v default_scope	Default scope
  */
 static inline void settings_init ( struct settings *settings,
 				   struct settings_operations *op,
 				   struct refcnt *refcnt,
-				   unsigned int tag_magic ) {
+				   struct settings_scope *default_scope ) {
 	INIT_LIST_HEAD ( &settings->siblings );
 	INIT_LIST_HEAD ( &settings->children );
 	settings->op = op;
 	settings->refcnt = refcnt;
-	settings->tag_magic = tag_magic;
+	settings->default_scope = default_scope;
 }
 
 /**
@@ -351,7 +368,7 @@ static inline void settings_init ( struct settings *settings,
 static inline void generic_settings_init ( struct generic_settings *generics,
 					   struct refcnt *refcnt ) {
 	settings_init ( &generics->settings, &generic_settings_operations,
-			refcnt, 0 );
+			refcnt, NULL );
 	INIT_LIST_HEAD ( &generics->list );
 }
 

src/interface/smbios/smbios_settings.c

@@ -27,15 +27,8 @@ FILE_LICENCE ( GPL2_OR_LATER );
 #include <ipxe/uuid.h>
 #include <ipxe/smbios.h>
 
-/** SMBIOS settings tag magic number */
-#define SMBIOS_TAG_MAGIC 0x5B /* "SmBios" */
-
-/**
- * Construct SMBIOS empty tag
- *
- * @ret tag		SMBIOS setting tag
- */
-#define SMBIOS_EMPTY_TAG ( SMBIOS_TAG_MAGIC << 24 )
+/** SMBIOS settings scope */
+static struct settings_scope smbios_settings_scope;
 
 /**
  * Construct SMBIOS raw-data tag
@@ -46,8 +39,7 @@ FILE_LICENCE ( GPL2_OR_LATER );
  * @ret tag		SMBIOS setting tag
  */
 #define SMBIOS_RAW_TAG( _type, _structure, _field )		\
-	( ( SMBIOS_TAG_MAGIC << 24 ) |				\
-	  ( (_type) << 16 ) |					\
+	( ( (_type) << 16 ) |					\
 	  ( offsetof ( _structure, _field ) << 8 ) |		\
 	  ( sizeof ( ( ( _structure * ) 0 )->_field ) ) )
 
@@ -60,8 +52,7 @@ FILE_LICENCE ( GPL2_OR_LATER );
  * @ret tag		SMBIOS setting tag
  */
 #define SMBIOS_STRING_TAG( _type, _structure, _field )		\
-	( ( SMBIOS_TAG_MAGIC << 24 ) |				\
-	  ( (_type) << 16 ) |					\
+	( ( (_type) << 16 ) |					\
 	  ( offsetof ( _structure, _field ) << 8 ) )
 
 /**
@@ -73,11 +64,8 @@ FILE_LICENCE ( GPL2_OR_LATER );
  */
 static int smbios_applies ( struct settings *settings __unused,
 			    struct setting *setting ) {
-	unsigned int tag_magic;
 
-	/* Check tag magic */
-	tag_magic = ( setting->tag >> 24 );
-	return ( tag_magic == SMBIOS_TAG_MAGIC );
+	return ( setting->scope == &smbios_settings_scope );
 }
 
 /**
@@ -93,18 +81,15 @@ static int smbios_fetch ( struct settings *settings __unused,
 			  struct setting *setting,
 			  void *data, size_t len ) {
 	struct smbios_structure structure;
-	unsigned int tag_magic;
 	unsigned int tag_type;
 	unsigned int tag_offset;
 	unsigned int tag_len;
 	int rc;
 
 	/* Split tag into type, offset and length */
-	tag_magic = ( setting->tag >> 24 );
 	tag_type = ( ( setting->tag >> 16 ) & 0xff );
 	tag_offset = ( ( setting->tag >> 8 ) & 0xff );
 	tag_len = ( setting->tag & 0xff );
-	assert ( tag_magic == SMBIOS_TAG_MAGIC );
 
 	/* Find SMBIOS structure */
 	if ( ( rc = find_smbios_structure ( tag_type, &structure ) ) != 0 )
@@ -170,10 +155,10 @@ static struct settings_operations smbios_settings_operations = {
 /** SMBIOS settings */
 static struct settings smbios_settings = {
 	.refcnt = NULL,
-	.tag_magic = SMBIOS_EMPTY_TAG,
 	.siblings = LIST_HEAD_INIT ( smbios_settings.siblings ),
 	.children = LIST_HEAD_INIT ( smbios_settings.children ),
 	.op = &smbios_settings_operations,
+	.default_scope = &smbios_settings_scope,
 };
 
 /** Initialise SMBIOS settings */
@@ -200,6 +185,7 @@ struct setting uuid_setting __setting ( SETTING_HOST ) = {
 	.tag = SMBIOS_RAW_TAG ( SMBIOS_TYPE_SYSTEM_INFORMATION,
 				struct smbios_system_information, uuid ),
 	.type = &setting_type_uuid,
+	.scope = &smbios_settings_scope,
 };
 
 /** Other SMBIOS named settings */
@@ -211,6 +197,7 @@ struct setting smbios_named_settings[] __setting ( SETTING_HOST_EXTRA ) = {
 					   struct smbios_system_information,
 					   manufacturer ),
 		.type = &setting_type_string,
+		.scope = &smbios_settings_scope,
 	},
 	{
 		.name = "product",
@@ -219,6 +206,7 @@ struct setting smbios_named_settings[] __setting ( SETTING_HOST_EXTRA ) = {
 					   struct smbios_system_information,
 					   product ),
 		.type = &setting_type_string,
+		.scope = &smbios_settings_scope,
 	},
 	{
 		.name = "serial",
@@ -227,6 +215,7 @@ struct setting smbios_named_settings[] __setting ( SETTING_HOST_EXTRA ) = {
 					   struct smbios_system_information,
 					   serial ),
 		.type = &setting_type_string,
+		.scope = &smbios_settings_scope,
 	},
 	{
 		.name = "asset",
@@ -235,5 +224,6 @@ struct setting smbios_named_settings[] __setting ( SETTING_HOST_EXTRA ) = {
 					   struct smbios_enclosure_information,
 					   asset_tag ),
 		.type = &setting_type_string,
+		.scope = &smbios_settings_scope,
 	},
 };

src/net/80211/net80211.c

@@ -208,7 +208,6 @@ struct setting net80211_ssid_setting __setting ( SETTING_NETDEV_EXTRA ) = {
 	.name = "ssid",
 	.description = "Wireless SSID",
 	.type = &setting_type_string,
-	.tag = NET80211_SETTING_TAG_SSID,
 };
 
 /** Whether to use active scanning
@@ -221,7 +220,6 @@ struct setting net80211_active_setting __setting ( SETTING_NETDEV_EXTRA ) = {
 	.name = "active-scan",
 	.description = "Actively scan for wireless networks",
 	.type = &setting_type_int8,
-	.tag = NET80211_SETTING_TAG_ACTIVE_SCAN,
 };
 
 /** The cryptographic key to use
@@ -234,7 +232,6 @@ struct setting net80211_key_setting __setting ( SETTING_NETDEV_EXTRA ) = {
 	.name = "key",
 	.description = "Wireless encryption key",
 	.type = &setting_type_string,
-	.tag = NET80211_SETTING_TAG_KEY,
 };
 
 /** @} */

src/net/dhcppkt.c

@@ -230,7 +230,8 @@ static int dhcppkt_settings_applies ( struct settings *settings,
 	struct dhcp_packet *dhcppkt =
 		container_of ( settings, struct dhcp_packet, settings );
 
-	return dhcppkt_applies ( dhcppkt, setting->tag );
+	return ( ( setting->scope == NULL ) &&
+		 dhcppkt_applies ( dhcppkt, setting->tag ) );
 }
 
 /**
@@ -299,6 +300,6 @@ void dhcppkt_init ( struct dhcp_packet *dhcppkt, struct dhcphdr *data,
 	dhcpopt_init ( &dhcppkt->options, &dhcppkt->dhcphdr->options,
 		       ( len - offsetof ( struct dhcphdr, options ) ),
 		       dhcpopt_no_realloc );
-	settings_init ( &dhcppkt->settings,
-			&dhcppkt_settings_operations, &dhcppkt->refcnt, 0 );
+	settings_init ( &dhcppkt->settings, &dhcppkt_settings_operations,
+			&dhcppkt->refcnt, NULL );
 }

src/net/netdev_settings.c

@@ -39,29 +39,13 @@ struct setting mac_setting __setting ( SETTING_NETDEV ) = {
 	.name = "mac",
 	.description = "MAC address",
 	.type = &setting_type_hex,
-	.tag = NETDEV_SETTING_TAG_MAC,
 };
 struct setting busid_setting __setting ( SETTING_NETDEV ) = {
 	.name = "busid",
 	.description = "Bus ID",
 	.type = &setting_type_hex,
-	.tag = NETDEV_SETTING_TAG_BUS_ID,
 };
 
-/**
- * Check applicability of network device setting
- *
- * @v settings		Settings block
- * @v setting		Setting
- * @ret applies		Setting applies within this settings block
- */
-static int netdev_applies ( struct settings *settings __unused,
-			    struct setting *setting ) {
-
-	return ( IS_NETDEV_SETTING_TAG ( setting->tag ) ||
-		 dhcpopt_applies ( setting->tag ) );
-}
-
 /**
  * Store value of network device setting
  *
@@ -134,7 +118,6 @@ static void netdev_clear ( struct settings *settings ) {
 
 /** Network device configuration settings operations */
 struct settings_operations netdev_settings_operations = {
-	.applies = netdev_applies,
 	.store = netdev_store,
 	.fetch = netdev_fetch,
 	.clear = netdev_clear,