[PATCH RFC 1/3] lib: Define autoboot options device_type helpers

[Samuel Mendoza-Jonas]

On 17/12/14 07:20, Geoff Levand wrote:
> Hi Samuel,
> 
Hi Geoff, thanks for the review!

> On Tue, 2014-12-16 at 10:43 +1100, Samuel Mendoza-Jonas wrote:
>> Signed-off-by: Samuel Mendoza-Jonas <sam.mj at au1.ibm.com>
>> ---
>>  lib/Makefile.am   |  1 +
>>  lib/types/types.c | 37 +++++++++++++++++++++++++++++++++++++
>>  lib/types/types.h | 14 ++++++++++++++
>>  3 files changed, 52 insertions(+)
>>  create mode 100644 lib/types/types.c
>>
>> diff --git a/lib/Makefile.am b/lib/Makefile.am
>> index fbf2ee2..b39cc9b 100644
>> --- a/lib/Makefile.am
>> +++ b/lib/Makefile.am
>> @@ -38,6 +38,7 @@ lib_libpbcore_la_SOURCES = \
>>  	lib/pb-config/pb-config.h \
>>  	lib/process/process.c \
>>  	lib/process/process.h \
>> +	lib/types/types.c \
>>  	lib/types/types.h \
>>  	lib/talloc/talloc.c \
>>  	lib/talloc/talloc.h \
>> diff --git a/lib/types/types.c b/lib/types/types.c
>> new file mode 100644
>> index 0000000..b76f766
>> --- /dev/null
>> +++ b/lib/types/types.c
>> @@ -0,0 +1,37 @@
>> +#include <string.h>
>> +#include <types/types.h>
>> +
>> +const char *device_type_name(enum device_type type)
>> +{
>> +	switch (type) {
>> +	case DEVICE_TYPE_DISK:
>> +		return "disk";
>> +	case DEVICE_TYPE_OPTICAL:
>> +		return "optical";
>> +	case DEVICE_TYPE_NETWORK:
>> +		return "network";
>> +	case DEVICE_TYPE_ANY:
>> +		return "any";
>> +	case DEVICE_TYPE_UNKNOWN:
>> +	default:
>> +		return "unknown";
> 
> Are we going to need localized strings for this?  Is 'any' OK
> for say the Chinese user?  Should we use a variable or
> preprocessor macro for these strings and the ones in
> find_device_type?  Maybe put the definition in types.h?

Good point - originally this was just used in dump_config(),
but ideally we'd like to use this or something similar in the UI.
I'll make sure these are localisation-ready.

> 
>> +}
>> +
>> +int find_device_type(const char *str)
> 
> I guess this should be returning a type of 'enum device_type'?

Yes
> 
>> +{
>> +	static const char *device_strings[4] = {
>> +		"network",
>> +		"disk",
>> +		"optical",
>> +		"any"
>> +	};
>> +	int i;
>> +
>> +	for (i = 0; i < 4; i++) {
>> +		if (!strncmp(str, device_strings[i], strlen(device_strings[i])))
>> +			return i;
> 
> I don't think you can rely on the compiler to assign
> the values to the enum device_type's for this to work.
> I think you need to do something like:
> 
>   if (!strncmp(str, "network", strlen("network"))
>     return DEVICE_TYPE_NETWORK;
> 

Yep, that's better. And looking at it now it would break quickly
if the enum were ever to change.

>> +	}
>> +
>> +	return i;
>> +}
>> diff --git a/lib/types/types.h b/lib/types/types.h
>> index aef1509..add81fc 100644
>> --- a/lib/types/types.h
>> +++ b/lib/types/types.h
>> @@ -13,6 +13,9 @@ enum device_type {
>>  	DEVICE_TYPE_UNKNOWN,
>>  };
>>  
>> +const char *device_type_name(enum device_type type);
>> +int find_device_type(const char *str);
>> +
>>  struct device {
>>  	char		*id;
>>  	enum device_type type;
>> @@ -118,6 +121,17 @@ struct boot_priority {
>>  	enum device_type	type;
>>  };
>>  
>> +struct autoboot_option {
>> +	enum {
>> +		BOOT_DEVICE_TYPE,
>> +		BOOT_DEVICE_UUID
>> +	} boot_type;
>> +	enum device_type	type;
>> +	char			*uuid;
>> +
>> +	struct list_item        list;
>> +};
>> +
>>  struct config {
>>  	bool			autoboot_enabled;
>>  	unsigned int		autoboot_timeout_sec;
> 


-- 
-----------
LTC Ozlabs
IBM