get_led_cmd() searches state_label[] for a matching name and returns -1 on no match, but its declared return type is enum led_state_t. All enumerators of that enum are non-negative, so the compiler is free to pick an unsigned underlying type (and does so with -fshort-enums, which several U-Boot targets enable). In that case -1 is reinterpreted as a large positive value and any subsequent comparison against -1 silently misbehaves.
In do_led() the returned value is stored into an enum led_state_t and dispatched through a switch that has no default arm and no case for the sentinel. When the user supplies an unknown state name the switch falls through, ret was already set to 0 by led_get_by_label(), and the command reports success without doing anything. Return LEDST_COUNT from get_led_cmd() on no match. LEDST_COUNT is already a valid enumerator, so this avoids all questions about the enum's underlying representation. In do_led(), when a state argument was actually supplied (argc > 2) and the lookup produced LEDST_COUNT, reject it with a diagnostic and CMD_RET_USAGE; the existing LEDST_COUNT switch arm continues to handle the "no state argument" case where it means "show current state". Signed-off-by: Naveen Kumar Chaudhary <[email protected]> --- cmd/led.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/cmd/led.c b/cmd/led.c index 296c07b3b38..d547276e480 100644 --- a/cmd/led.c +++ b/cmd/led.c @@ -27,7 +27,7 @@ enum led_state_t get_led_cmd(char *var) return i; } - return -1; + return LEDST_COUNT; } static int show_led_state(struct udevice *dev) @@ -84,6 +84,10 @@ int do_led(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) return list_leds(); cmd = argc > 2 ? get_led_cmd(argv[2]) : LEDST_COUNT; + if (argc > 2 && cmd == LEDST_COUNT) { + printf("Unknown LED state '%s'\n", argv[2]); + return CMD_RET_USAGE; + } if (cmd == LEDST_BLINK) { if (argc < 4) return CMD_RET_USAGE; -- 2.43.0
