[PATCH 1/3] lib/types : Adds function to return struct config as string

[Samuel Mendoza-Jonas]

On Thu, Jun 02, 2016 at 05:56:56AM -0400, Nayna Jain wrote:
> Two new functions added to return ipmi boot configuration
> and nvram boot configuration separately as string.

At first I thought this was a reimplementation of dump_config(), but I
see you're concatenating the config and nvram structs into a string so
they can be hashed in the later patches. Is this the best way to do
this?

> 
> Signed-off-by: Nayna Jain <nayna at linux.vnet.ibm.com>
> ---
>  lib/types/types.c | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  lib/types/types.h |  3 +++
>  2 files changed, 78 insertions(+)
> 
> diff --git a/lib/types/types.c b/lib/types/types.c
> index 63045e1..2f924f1 100644
> --- a/lib/types/types.c
> +++ b/lib/types/types.c
> @@ -1,4 +1,6 @@
>  #include <string.h>
> +#include <stdlib.h>
> +#include <log/log.h>
>  #include <types/types.h>
>  #include <i18n/i18n.h>
>  
> @@ -75,3 +77,76 @@ enum device_type find_device_type(const char *str)
>  
>  	return DEVICE_TYPE_UNKNOWN;
>  }
> +
> +unsigned char *get_ipmi_boot_policy_as_string(const struct config *config)
> +{
> +        unsigned char * input_data;

Please format this (and similar declarations) as

        unsigned char *input_data;

> +
> +        input_data = talloc_asprintf(config, "%04x%s", config->ipmi_bootdev,
> +			config->ipmi_bootdev_persistent ? "1" : "0");
> +        pb_log("Input data is %s, %d, %d\n", input_data, sizeof(input_data),
> +			strlen(input_data));
> +
> +        return input_data;
> +}
> +
> +unsigned char *get_nvram_boot_policy_as_string(const struct config* config)
> +{
> +
> +        char * input_data = NULL;
> +        char * autoboot_device = NULL;
> +        unsigned int i = 0;

In Petitboot we style this largest-to-smallest to make it easier to
read, for example:

        char *autoboot_device = NULL;
        char *input_data = NULL;
        unsigned int i = 0;

Not a huge deal here but helps a lot in more noisy functions :)

> +
> +        input_data = talloc_asprintf(config, "%s%d%s%s%s%s",
> +			config->autoboot_enabled ? "1": "0",
> +                	config->autoboot_timeout_sec,
> +			config->safe_mode ? "1" : "0",
> +			config->allow_writes ? "1" : "0",
> +			config->disable_snapshots? "1" : "0",
> +                	config->lang ? config->lang : "0");

Please use tabs instead of spaces (two of the lines here stand out)

> +        for (i = 0; i < config->n_autoboot_opts; i++)
> +        {

Please format 'for' and 'if' braces like so:

	if (cond) {
		// stuff
	}

> +                if (config->autoboot_opts[i].boot_type == BOOT_DEVICE_TYPE)
> +                        autoboot_device = device_type_name(
> +					config->autoboot_opts[i].type);
> +                else
> +                        autoboot_device = config->autoboot_opts[i].uuid;
> +                input_data = talloc_asprintf_append(input_data, "%s",
> +				autoboot_device);
> +        }
> +
> +
> +        for (i =0; i < config->network.n_interfaces; i++)

Also please be consistent with spacing (i = 0 rather than i =0)

> +        {
> +                struct interface_config *iconf = config->network.interfaces[i];
> +                input_data = talloc_asprintf_append(input_data,
> +				"%02x:%02x:%02x:%02x:%02x:%02x",
> +				iconf->hwaddr[0], iconf->hwaddr[1],
> +				iconf->hwaddr[2], iconf->hwaddr[3],
> +                                iconf->hwaddr[4], iconf->hwaddr[5]);
> +
> +                input_data = talloc_asprintf_append(input_data,"%s",
> +				iconf->ignore ? "1" : "0");
> +                if (iconf->method == CONFIG_METHOD_DHCP) {
> +                        input_data = talloc_asprintf_append(input_data,
> +					"%s", "dhcp");
> +
> +                } else if (iconf->method == CONFIG_METHOD_STATIC) {
> +                        input_data = talloc_asprintf_append(input_data,
> +					"%s%s%s%s", "static",
> +					iconf->static_config.address,
> +					iconf->static_config.gateway,
> +                                        iconf->static_config.url);
> +                }
> +
> +        }
> +
> +        for (i = 0; i < config->network.n_dns_servers; i++)
> +                input_data = talloc_asprintf_append(input_data, "%s",
> +				config->network.dns_servers[i]);
> +
> +        pb_log("input data is %s, %d, %d\n", input_data, sizeof(input_data),
> +			strlen(input_data));
> +        return input_data;
> +
> +}
> diff --git a/lib/types/types.h b/lib/types/types.h
> index 6ff4620..e6e1a78 100644
> --- a/lib/types/types.h
> +++ b/lib/types/types.h
> @@ -167,4 +167,7 @@ struct config {
>  	bool			debug;
>  };
>  
> +unsigned char *get_ipmi_boot_policy_as_string(const struct config *conf);
> +unsigned char *get_nvram_boot_policy_as_string(const struct config *conf);
> +
>  #endif /* _TYPES_H */
> -- 
> 2.5.0
> 
> _______________________________________________
> Petitboot mailing list
> Petitboot at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/petitboot