On Thu, 16 Jan 2020 17:35:32 +0100
Thomas Huth <th...@redhat.com> wrote:
> 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...
I'll split it into separate patch
>
> > 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"
xxx is variable so options are
1 build this part of CLI dynamically
2 mostly duplicate testcase function and include per target size there
3 make up a test data structure and pass that to test cases
Given simplicity of current testcases, I'd prefer continue with
passing CLI as testcase data (option #1).
>
> there? So using a GString seems overkill to me here.
>
> Thomas
