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: <20220408092803.3188-1-h0tc0d3@gmail.com> <4o2YD5id7zhK5g__CUK3Yf-6Dj459XQ0qsydW6wKHdco3YYtl78BqGUfXvQ9UO0AZoAsMPfhFB0jw1BFldiFDcbZXmGgsobuAoTMWmoDqmE=@emersion.fr>
 <CAP+8YyED70CSqVRaB7JgrAtK-yvBx5EsvrrdR7EpvOG2AVhskQ@mail.gmail.com>
 <QfKpxmkE_cy9wt5VJruw_TSdnl5KceKM8BxJGmZSBs-KiaRwIYfgc8h_-5h7Wmj6G-NtUJ3A88V5pzPvZuLlpkK-oRO5pSjeTxwHcZWlogs=@emersion.fr>
 <CAD5ugGD6QzCUqk7_EVwH9Cc6PQtx_VfjVRWzzP9uKR5tkGh1RQ@mail.gmail.com> <436116c9-6ce4-87cc-b1cd-f44b72fcfb35@amd.com>
In-Reply-To: <436116c9-6ce4-87cc-b1cd-f44b72fcfb35@amd.com>
From:   Grigory Vasilyev <h0tc0d3@gmail.com>
Date:   Sun, 10 Apr 2022 03:51:39 +0300
Message-ID: <CAD5ugGDrDrWWf1xMKTy=56H1AR8cyXMV41f1X8P=HVtYY6idDA@mail.gmail.com>
Subject: Re: [PATCH] drm/amdgpu: Fix NULL pointer dereference
To:     =?UTF-8?Q?Christian_K=C3=B6nig?= <christian.koenig@amd.com>
Cc:     Simon Ser <contact@emersion.fr>,
        Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>,
        Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>,
        Melissa Wen <mwen@igalia.com>,
        Alex Deucher <alexander.deucher@amd.com>,
        "Pan, Xinhui" <Xinhui.Pan@amd.com>,
        David Airlie <airlied@linux.ie>,
        Daniel Vetter <daniel@ffwll.ch>,
        =?UTF-8?Q?Michel_D=C3=A4nzer?= <mdaenzer@redhat.com>,
        Evan Quan <evan.quan@amd.com>,
        Sean Paul <seanpaul@chromium.org>,
        Qingqing Zhuo <qingqing.zhuo@amd.com>,
        amd-gfx mailing list <amd-gfx@lists.freedesktop.org>,
        ML dri-devel <dri-devel@lists.freedesktop.org>,
        LKML <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Christian K=C3=B6nig, Simon Ser In fact, the code looks strange, we return
the return code, but for some reason we also write false and 0. In my
opinion, the caller should do this.
Of course, you are right, but I look from the position that nothing
should fall in the user system. There may be strange errors that occur
periodically under certain conditions and do not affect developers.
Also, the user will not always be able to correctly write a bug report.

Regards, Grigory.

=D0=BF=D1=82, 8 =D0=B0=D0=BF=D1=80. 2022 =D0=B3. =D0=B2 17:30, Christian K=
=C3=B6nig <christian.koenig@amd.com>:
>
> Am 08.04.22 um 15:21 schrieb Grigory Vasilyev:
> > Simon Ser and Bas Nieuwenhuizen, do you understand that you are
> > proposing to make the code less safe in the future? In the future,
> > someone might rewrite the code and we'll get an error.
>
> Which is perfectly fine.
>
> See error handling is to handle userspace or hardware errors and *not*
> coding errors.
>
> Testing all pointers for NULL without any reason is not defensive, but
> rather the exactly opposite since it helps hiding real bugs.
>
> Regards,
> Christian.
>
> >
> > =D0=BF=D1=82, 8 =D0=B0=D0=BF=D1=80. 2022 =D0=B3. =D0=B2 14:48, Simon Se=
r <contact@emersion.fr>:
> >> On Friday, April 8th, 2022 at 13:28, Bas Nieuwenhuizen <bas@basnieuwen=
huizen.nl> wrote:
> >>
> >>> On Fri, Apr 8, 2022 at 12:01 PM Simon Ser contact@emersion.fr wrote:
> >>>
> >>>> Is amdgpu_display_get_fb_info ever called with NULL tiling_flags/tmz=
_surface?
> >>>> If not, there's no point in adding NULL checks.
> >>> It isn't called with NULL anywhere, the NULL checks that already exis=
t
> >>> seem redundant.
> >> Grigory, would be be willing to submit a v2 which removes the unnecess=
ary
> >> NULL checks?
>