Jiang Xin writes:
> +static void print_highlight_menu_stuff(struct menu_stuff *stuff, int
> **chosen)
> +{
> + struct string_list menu_list = STRING_LIST_INIT_DUP;
> + struct strbuf menu = STRBUF_INIT;
> + int i;
> +
> + if (MENU_STUFF_TYPE_MENU_ITEM == stuff->type) {
> + ...
> + } else if (MENU_STUFF_TYPE_STRING_LIST == stuff->type) {
> + ...
> + }
This is better to write as:
switch (stuff->type) {
default:
die("programming error");
case MENU_STUFF_TYPE_MENU_ITEM:
...
break;
case MENU_STUFF_TYPE_STRING_LIST:
...
break;
}
Besides, there is no good reason to write an equality comparison
between constant and variable in that order (people call it a "Yoda
condition"); do "var == const" if you must.
Also please fix this one:
> + for_each_string_list_item(item, (struct string_list
> *)stuff->stuff) {
> + if ((*chosen)[i] < 0)
> + (*chosen)[i] = 0;
> + strbuf_addf(&menu, "%s%2d: %s", (*chosen)[i] ? "*" : "
> ", ++i, item->string);
Because the evaluation order of function arguments are not defined
(not left to right; these are comma-expressions),
(*chosen)[i] ? "*" : " "
may use the original value of "i", or value after increment the
evaluation of
++i
left in "i".
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
