[PATCH 2/2] ui/ncurses: Add device hierarchy

[Jeremy Kerr]

> +
> +	/* Check if device header exists */

It might be cleaner to move this code (to find or create the header
item) to a separate function.

> +	for (j=0; j < cui->main->item_count; j++){
> +		struct pmenu_item *item = item_userptr(cui->main->items[j]);
> +		struct cui_opt_data *data = item->data;
> +		/* boot entries will have this defined */
> +		if(!data || data->opt)
> +			continue;
> +		if(!strcmp(data->name,opt->device_id)){
> +			pb_debug("%s: %s fits under %s\n",__func__,
> +				 opt->name, opt->device_id);
> +			newdev = 0;
> +			break;
> +		}
> +	}
> +
> +	/* Create a dummy pmenu_item to represent the dev */
> +	if (newdev) {
> +		char *buf = talloc_array(cui->main, char, sizeof(char)*256);

May as well just put this on the stack:

		char buf[256];

- and then you don't need to repeat the size elsewhere in the function,
but use sizeof(buf) instead.

> +		struct system_info *sys = cui->sysinfo;
> +		int matched;

bool

> +		switch (dev->type) {
> +		case DEVICE_TYPE_OPTICAL:
> +		case DEVICE_TYPE_DISK:
> +		{

no need for the braces if you don't have any declared variables (and I'd
prefer declaring them in an outer scope anyway).

> +	/* All actual boot entries are 'tabbed' across */
> +	if(!opt->name)
> +		len = sizeof("    Unknown Name");
> +	else
> +		len = strlen(opt->name) + sizeof(char)*5;
> +	shift = talloc_array(cui->main, char, len);
> +	snprintf(shift,len,"    %s",opt->name ? opt->name : "Unknown Name");

can be replaced with:

   talloc_asprintf(cui->main, "    %s", opt->name ? : "Unknown Name");

Might also be good to make the indent a const char * variable, so you
don't have to repeat yourself.

> @@ -500,6 +598,7 @@ static void cui_device_remove(struct device *dev, void *arg)
>  {
>  	struct cui *cui = cui_from_arg(arg);
>  	int result;
> +	unsigned int ind;

Nitpick: we've generally used 'i' as a loop counter; then you could use
 'item' for the pmenu_item variable.

Cheers,


Jeremy