From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id DC328A0547 for ; Wed, 11 Aug 2021 14:56:06 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id C64E540042; Wed, 11 Aug 2021 14:56:06 +0200 (CEST) Received: from mail-ot1-f54.google.com (mail-ot1-f54.google.com [209.85.210.54]) by mails.dpdk.org (Postfix) with ESMTP id 4960F40042 for ; Wed, 11 Aug 2021 14:56:05 +0200 (CEST) Received: by mail-ot1-f54.google.com with SMTP id a7-20020a9d5c870000b029050333abe08aso3136441oti.13 for ; Wed, 11 Aug 2021 05:56:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=iol.unh.edu; s=unh-iol; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=NcZAeR5R/v7uRGcjMmJLckpFawGgMzdVL9fx5IwONZU=; b=U/ZoPZK/gLiUCBahLpGk3cj1fRazyZtAYlPzX8trYe3lx/jBxBgH9ApMCs/C3iXzww 7AO7iNxCIEDn/gW8O696Qsj14YfEMwp+bwnhQehSW5rTkn92Wswfd5j3bCH9dZw9GmPR sZkM94SyurzYW4TYtN4LjuxqcrRy98+ZgMljA= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=NcZAeR5R/v7uRGcjMmJLckpFawGgMzdVL9fx5IwONZU=; b=UXE0+Bf+9UBu63OjkUQQVmASXxjL8Fo4h8t56eJJM9nJ6BmhO6hbp6l0XBeXXO4DV4 XKkc3tIw3gKwoWH2gufuu30LyOfE/aqOS7Qs4kEU7uOCxS3+0DzsNnCFIkBpKaAE/0EW KtataVOGTjYbVH4wo3NPUxzWdun3I7wtNbEO9XySxTat2rgLmUWzKx4w9shSnAIZejWq lqQfvbk3J0aMCfl2DfTuV0NHUfp8Bdnw4DQYNJcAe/mMqeIGrVMZfZnYBMc/zHDYqIcz Q79VJNaRbGL+WpGBLZMRAGDP2wX+A3M1HiWmirwpOZL3oguGdhvqSJcY+hzQ1kJ1HG7h na0w== X-Gm-Message-State: AOAM533009Zf7iYuJzMIeFXDFdf+4UHvMNcDhAK5PeLcEyAJuB0IoEKS H437PGE8CkbxZeml1Lw2UfEdskZlkLHmaUwTsxeh0g== X-Google-Smtp-Source: ABdhPJzvjGlaelSrYcQ4snpd2SWTszCiAemnT8NZ4SXT8YwvKa/4I5nMFQKUKutqj3C471AKvM2aoODrSq/GQ3LLeOc= X-Received: by 2002:a9d:2903:: with SMTP id d3mr11307253otb.117.1628686564599; Wed, 11 Aug 2021 05:56:04 -0700 (PDT) MIME-Version: 1.0 References: <20210810173950.98785-1-ohilyard@iol.unh.edu> In-Reply-To: From: Owen Hilyard Date: Wed, 11 Aug 2021 08:55:29 -0400 Message-ID: To: Christian Ehrhardt Cc: dpdk stable , David Marchand Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Content-Filtered-By: Mailman/MimeDel 2.1.29 Subject: Re: [dpdk-stable] [PATCH 19.11] tests/cmdline: fix memory leaks X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: stable-bounces@dpdk.org Sender: "stable" Sorry about that, I'll go take a second look and send in a V2. On Wed, Aug 11, 2021 at 7:18 AM Christian Ehrhardt < christian.ehrhardt@canonical.com> wrote: > On Wed, Aug 11, 2021 at 11:45 AM Christian Ehrhardt > wrote: > > > > On Wed, Aug 11, 2021 at 8:43 AM Christian Ehrhardt > > wrote: > > > > > > On Tue, Aug 10, 2021 at 7:40 PM wrote: > > > > > > > > From: Owen Hilyard > > > > > > > > [ upstream commit ca7204b921c2f328ab1222772af40922970e7c4b ] > > > > > > > > Fixes for a few memory leaks in the cmdline_autotest unit test. > > > > > > Thank you for the backport - applied to the preliminary 19.11 branch > > > for 19.11.10 > > > > Actually this causes > > ../app/test/test_cmdline_lib.c: In function =E2=80=98test_cmdline_socke= t_fns=E2=80=99: > > ../app/test/test_cmdline_lib.c:138:8: warning: assignment to =E2=80=98s= truct > > cmdline *=E2=80=99 from =E2=80=98int=E2=80=99 makes pointer from intege= r without a cast > > [-Wint-conversion] > > 138 | cl =3D cmdline_stdin_new(NULL, "prompt") !=3D NULL); > > | ^ > > ../app/test/test_cmdline_lib.c:138:51: error: expected =E2=80=98;=E2=80= =99 before =E2=80=98)=E2=80=99 > token > > 138 | cl =3D cmdline_stdin_new(NULL, "prompt") !=3D NULL); > > | ^ > > ../app/test/test_cmdline_lib.c:138:51: error: expected statement > > before =E2=80=98)=E2=80=99 token > > > > It has various issues, see below: > > > > > > All of the leaks were related to not freeing the commandline struct > > > > after testing had completed. > > > > > > > > Fixes: dbb860e03e ("cmdline: tests") > > > > > > > > Signed-off-by: Owen Hilyard > > > > Reviewed-by: David Marchand > > > > --- > > > > app/test/test_cmdline_lib.c | 43 > ++++++++++++++++++++++++++----------- > > > > 1 file changed, 31 insertions(+), 12 deletions(-) > > > > > > > > diff --git a/app/test/test_cmdline_lib.c > b/app/test/test_cmdline_lib.c > > > > index a856a9713..c7ada9ddd 100644 > > > > --- a/app/test/test_cmdline_lib.c > > > > +++ b/app/test/test_cmdline_lib.c > > > > @@ -62,10 +62,12 @@ test_cmdline_parse_fns(void) > > > > if (cmdline_complete(&cl, "buffer", &i, NULL, sizeof(dst)) > >=3D 0) > > > > goto error; > > > > > > > > + cmdline_free(&cl); > > > > return 0; > > > > > > > > error: > > > > printf("Error: function accepted null parameter!\n"); > > > > + cmdline_free(&cl); > > > > return -1; > > > > } > > > > > > > > @@ -131,32 +133,43 @@ static int > > > > test_cmdline_socket_fns(void) > > > > { > > > > cmdline_parse_ctx_t ctx; > > > > + struct cmdline* cl; > > > > > > > > - if (cmdline_stdin_new(NULL, "prompt") !=3D NULL) > > > > + cl =3D cmdline_stdin_new(NULL, "prompt") !=3D NULL); > > > > Changed to an assignment but kept the comparison of the if =3D> failing= . > > > > > > + if (cl !=3D NULL) > > > > goto error; > > > > - if (cmdline_stdin_new(&ctx, NULL) !=3D NULL) > > > > + cl =3D cmdline_stdin_new(&ctx, NULL); > > > > + if (cl !=3D NULL) > > > > ^^ all those add whitespace damage replacing a tab with spaces > > > > > > I have fixed all those into > > > https://github.com/cpaelzer/dpdk-stable-queue/commit/4c36f7b56fb4bc12b9d1= 9ca21db4207298afead0 > > Even with that if fails like > > [ 1044s] ../app/test/test_cmdline_lib.c: In function =E2=80=98test_cmdlin= e_fns=E2=80=99: > [ 1044s] ../app/test/test_cmdline_lib.c:227:2: error: =E2=80=98tmp2=E2=80= =99 may be > used uninitialized in this function [-Werror=3Dmaybe-uninitialized] > [ 1044s] 227 | cmdline_free(tmp2); > [ 1044s] | ^~~~~~~~~~~~~~~~~~ > > I'll de-queue it from 19.11 for now and would appreciate it if you > could check it more thoroughly than I did. > > > But please give all of it a second look > > > > > > goto error; > > > > - if (cmdline_file_new(NULL, "prompt", "/dev/null") !=3D NULL= ) > > > > + cl =3D cmdline_file_new(NULL, "prompt", "/dev/null"); > > > > + if (cl !=3D NULL) > > > > goto error; > > > > - if (cmdline_file_new(&ctx, NULL, "/dev/null") !=3D NULL) > > > > + cl =3D cmdline_file_new(&ctx, NULL, "/dev/null"); > > > > + if (cl !=3D NULL) > > > > goto error; > > > > - if (cmdline_file_new(&ctx, "prompt", NULL) !=3D NULL) > > > > + cl =3D cmdline_file_new(&ctx, "prompt", NULL); > > > > + if (cl !=3D NULL) > > > > goto error; > > > > - if (cmdline_file_new(&ctx, "prompt", "-/invalid/~/path") != =3D > NULL) { > > > > + cl =3D cmdline_file_new(&ctx, "prompt", "-/invalid/~/path"); > > > > + if (cl !=3D NULL) { > > > > printf("Error: succeeded in opening invalid file fo= r > reading!"); > > > > + cmdline_free(cl); > > > > return -1; > > > > } > > > > - if (cmdline_file_new(&ctx, "prompt", "/dev/null") =3D=3D NU= LL) { > > > > + cl =3D cmdline_file_new(&ctx, "prompt", "/dev/null"); > > > > + if (cl =3D=3D NULL) { > > > > printf("Error: failed to open /dev/null for > reading!"); > > > > + cmdline_free(cl); > > > > return -1; > > > > } > > > > > > > > /* void functions */ > > > > cmdline_stdin_exit(NULL); > > > > - > > > > + cmdline_free(cl); > > > > return 0; > > > > error: > > > > printf("Error: function accepted null parameter!\n"); > > > > + cmdline_free(cl); > > > > return -1; > > > > } > > > > > > > > @@ -164,16 +177,18 @@ static int > > > > test_cmdline_fns(void) > > > > { > > > > cmdline_parse_ctx_t ctx; > > > > - struct cmdline cl, *tmp; > > > > + struct cmdline cl, *tmp, *tmp2; > > > > > > > > memset(&ctx, 0, sizeof(ctx)); > > > > tmp =3D cmdline_new(&ctx, "test", -1, -1); > > > > if (tmp =3D=3D NULL) > > > > goto error; > > > > > > > > - if (cmdline_new(NULL, "prompt", 0, 0) !=3D NULL) > > > > + tmp2 =3D cmdline_new(NULL, "prompt", 0, 0); > > > > + if (tmp2 !=3D NULL) > > > > goto error; > > > > - if (cmdline_new(&ctx, NULL, 0, 0) !=3D NULL) > > > > + tmp2 =3D cmdline_new(&ctx, NULL, 0, 0); > > > > + if (tmp2 !=3D NULL) > > > > goto error; > > > > if (cmdline_in(NULL, "buffer", CMDLINE_TEST_BUFSIZE) >=3D 0= ) > > > > goto error; > > > > @@ -202,14 +217,18 @@ test_cmdline_fns(void) > > > > if (memcmp(&cl, tmp, sizeof(cl))) goto mismatch; > > > > > > > > cmdline_free(tmp); > > > > - > > > > + cmdline_free(tmp2); > > > > return 0; > > > > > > > > error: > > > > printf("Error: function accepted null parameter!\n"); > > > > + cmdline_free(tmp); > > > > + cmdline_free(tmp2); > > > > return -1; > > > > mismatch: > > > > printf("Error: data changed!\n"); > > > > + cmdline_free(tmp); > > > > + cmdline_free(tmp2); > > > > return -1; > > > > } > > > > > > > > -- > > > > 2.30.2 > > > > > > > > > > > > > -- > > > Christian Ehrhardt > > > Staff Engineer, Ubuntu Server > > > Canonical Ltd > > > > > > > > -- > > Christian Ehrhardt > > Staff Engineer, Ubuntu Server > > Canonical Ltd > > > > -- > Christian Ehrhardt > Staff Engineer, Ubuntu Server > Canonical Ltd >