Re: [LTP] [PATCH v6 2/4] tst_test.c: Add tst_cmd_present check if a command is present

[Cyril Hrubis <chrubis@suse.cz>]

Hi!
> 1) tst_brk for not checked command at the end.
> 2) tst_brk for NULL otherwise we get segfault. Maybe check also for empty
> command before attempting to do anything else.

Technically the empty command will end up in the tst_brk() at the end,
so I would add only the NULL check.

> index bf92e6cc92..cf67c2a9f0 100644
> --- lib/tst_test.c
> +++ lib/tst_test.c
> @@ -1379,12 +1379,16 @@ bool tst_cmd_present(const char *cmd)
>  {
>  	struct tst_cmd *pcmd = tst_test->needs_cmds;
>  
> +	if (!cmd || cmd[0] == '\0')
> +		tst_brk(TBROK, "Invalid cmd");
> +
>  	while (pcmd->cmd) {
>  		if (!strcmp(pcmd->cmd, cmd))
>  			return pcmd->present;
>  
>  		pcmd++;
>  	}
> +	tst_brk(TBROK, "'%s' not checked", cmd);
>  	return false;
>  }
>  
> 
> 3) Shouldn't we use tst_brk_() to point to the code in the test instead of
> tst_brk()? If yes, we probably want to have tst_cmd_present_() as library
> internal (static, with const char *file, const int lineno parameters) which
> calls tst_brk_() and tst_cmd_present() as public (the only with signature in
> header).

I do not think that this is that important to have, the
tst_cmd_present() calls are going to be quite rare and hence easy to
locate in the sources.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp