Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20221028144241.634012-1-her0gyugyu@gmail.com> <CABVgOSmtD3GRV3hDoMw9EgMG4DPFWWK1SEDJT_yGDs7-CnwveA@mail.gmail.com>
In-Reply-To: <CABVgOSmtD3GRV3hDoMw9EgMG4DPFWWK1SEDJT_yGDs7-CnwveA@mail.gmail.com>
From:   Daniel Latypov <dlatypov@google.com>
Date:   Mon, 31 Oct 2022 13:47:41 -0700
Message-ID: <CAGS_qxqWcD6wsncncqtFR00QO1QcLPm3e1Vzjto3ah-NoE3+jg@mail.gmail.com>
Subject: Re: [PATCH] kunit: alloc_string_stream_fragment error handling bug fix
To:     David Gow <davidgow@google.com>
Cc:     "YoungJun.park" <her0gyugyu@gmail.com>,
        Brendan Higgins <brendan.higgins@linux.dev>,
        linux-kselftest@vger.kernel.org, kunit-dev@googlegroups.com,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Sat, Oct 29, 2022 at 8:20 PM 'David Gow' via KUnit Development
<kunit-dev@googlegroups.com> wrote:
>
> On Fri, Oct 28, 2022 at 10:43 PM YoungJun.park <her0gyugyu@gmail.com> wro=
te:
> >
> > When it fails to allocate fragment, it does not free and return error.
> > And check the pointer inappropriately.
> >
> > Signed-off-by: YoungJun.park <her0gyugyu@gmail.com>
> > ---
>
> Thanks! As Ma=C3=ADra points out, the added kunit_kfree() call isn't
> strictly necessary, though it definitely doesn't hurt (and it's
> probably a nice thing to free memory early if we're already in a
> pretty dire memory situation). So I think it's an improvement.
>
> The IS_ERR check is definitely a fix, though.

Note: the IS_ERR check was fixed already in
https://patchwork.kernel.org/project/linux-kselftest/patch/Y0kt1aCTHO4r2CmL=
@kili/
That change has made its way into torvalds/master.
So we could rebase this patch and reword it to talk just about the
early kfree().

Re free memory early:
It'll save us sizeof(struct string_stream_fragment) + sizeof(struct
kunit_resource), i.e. 24 + 56 =3D 80 bytes (on UML/x86_64).
So it's not much, but I guess it could help in edge cases.

Daniel