On 15/01/2020 16.07, Igor Mammedov wrote:
> Use GString to pass argument to make_cli() so that it would be easy
> to dynamically change test case arguments from main(). The follow up
> patch will use it to change RAM size options depending on target.
>
> While at it cleanup 'cli' freeing, using g_autofree annotation.
Hmm, I'd use g_autofree for new code or do it in a separate cleanup
patch, but doing this here distracts quite a bit from the real changes
that you are doing...
> Signed-off-by: Igor Mammedov <imamm...@redhat.com>
> ---
> PS:
> made as a separate patch so it won't clutter followup testcase changes.
>
> CC: th...@redhat.com
> CC: lviv...@redhat.com
> ---
> tests/qtest/numa-test.c | 38 ++++++++++++++------------------------
> 1 file changed, 14 insertions(+), 24 deletions(-)
>
> diff --git a/tests/qtest/numa-test.c b/tests/qtest/numa-test.c
> index 17dd807..a696dfd 100644
> --- a/tests/qtest/numa-test.c
> +++ b/tests/qtest/numa-test.c
> @@ -14,16 +14,16 @@
> #include "qapi/qmp/qdict.h"
> #include "qapi/qmp/qlist.h"
>
> -static char *make_cli(const char *generic_cli, const char *test_cli)
> +static char *make_cli(const GString *generic_cli, const char *test_cli)
> {
> - return g_strdup_printf("%s %s", generic_cli ? generic_cli : "",
> test_cli);
> + return g_strdup_printf("%s %s", generic_cli->str, test_cli);
> }
[...]
> @@ -539,11 +529,11 @@ static void pc_hmat_erange_cfg(const void *data)
>
> int main(int argc, char **argv)
> {
> - const char *args = NULL;
> + g_autoptr(GString) args = g_string_new("");
I think g_string_new(NULL) would be better?
> const char *arch = qtest_get_arch();
>
> if (strcmp(arch, "aarch64") == 0) {
> - args = "-machine virt";
> + g_string_append(args, " -machine virt")> }
Is this really required? Looking at your next patch, you could also
simply do
args = " -object memory-backend-ram,id=ram,size=xxxM"
there? So using a GString seems overkill to me here.
Thomas
