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;
Feedback-ID: i8771445c:Fastmail
Date:   Wed, 7 Sep 2022 10:39:45 +0200
From:   Maxime Ripard <maxime@cerno.tech>
To:     Jani Nikula <jani.nikula@linux.intel.com>
Cc:     Geert Uytterhoeven <geert@linux-m68k.org>,
        Ben Skeggs <bskeggs@redhat.com>,
        David Airlie <airlied@linux.ie>, Chen-Yu Tsai <wens@csie.org>,
        Thomas Zimmermann <tzimmermann@suse.de>,
        Lyude Paul <lyude@redhat.com>,
        Philipp Zabel <p.zabel@pengutronix.de>,
        Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
        Rodrigo Vivi <rodrigo.vivi@intel.com>,
        Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>,
        Jernej Skrabec <jernej.skrabec@gmail.com>,
        Samuel Holland <samuel@sholland.org>,
        Karol Herbst <kherbst@redhat.com>,
        Noralf =?utf-8?Q?Tr=C3=B8nnes?= <noralf@tronnes.org>,
        Emma Anholt <emma@anholt.net>, Daniel Vetter <daniel@ffwll.ch>,
        Joonas Lahtinen <joonas.lahtinen@linux.intel.com>,
        Hans de Goede <hdegoede@redhat.com>,
        Linux ARM <linux-arm-kernel@lists.infradead.org>,
        Phil Elwell <phil@raspberrypi.com>,
        Intel Graphics Development <intel-gfx@lists.freedesktop.org>,
        Dave Stevenson <dave.stevenson@raspberrypi.com>,
        DRI Development <dri-devel@lists.freedesktop.org>,
        Dom Cobley <dom@raspberrypi.com>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Nouveau Dev <nouveau@lists.freedesktop.org>,
        linux-sunxi@lists.linux.dev,
        Mateusz Kwiatkowski <kfyatek+publicgit@gmail.com>
Subject: Re: [PATCH v2 14/41] drm/modes: Move named modes parsing to a
 separate function
Message-ID: <20220907083945.3cqmz765vmnjt3ol@houat>
References: <20220728-rpi-analog-tv-properties-v2-0-459522d653a7@cerno.tech>
 <20220728-rpi-analog-tv-properties-v2-14-459522d653a7@cerno.tech>
 <CAMuHMdV9wVgHFfwHoqtBoYzJDnjDmKTfaZkAKvTVKh1Y-2x1pA@mail.gmail.com>
 <87czcidnb8.fsf@intel.com>
 <20220830120330.6f5f22d35gu7cbr3@houat>
 <875yi9etuw.fsf@intel.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
        protocol="application/pgp-signature"; boundary="qxovco4c3ldtue3z"
Content-Disposition: inline
In-Reply-To: <875yi9etuw.fsf@intel.com>
Precedence: bulk


--qxovco4c3ldtue3z
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Hi,

On Tue, Aug 30, 2022 at 04:36:23PM +0300, Jani Nikula wrote:
> On Tue, 30 Aug 2022, Maxime Ripard <maxime@cerno.tech> wrote:
> > Hi,
> >
> > On Tue, Aug 30, 2022 at 01:43:07PM +0300, Jani Nikula wrote:
> >> On Tue, 30 Aug 2022, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> >> > On Mon, Aug 29, 2022 at 3:13 PM Maxime Ripard <maxime@cerno.tech> wr=
ote:
> >> >> +#define STR_STRICT_EQ(str, len, cmp) \
> >> >> +       ((strlen(cmp) =3D=3D len) && !strncmp(str, cmp, len))
> >> >
> >> > This is not part of the move, but newly added.
> >>=20
> >> The same construct is also duplicated elsewhere in the series, and I
> >> kept being confused by it.
> >
> > I'm not sure what is confusing, but I can add a comment if needed.
>=20
> STR_STRICT_EQ() is what's confusing. I have to look at the
> implementation to understand what it means. What does "strict" string
> equality mean?

Same length, same sequence of characters

> >
> >> The above is precisely the same as:
> >>=20
> >> 	str_has_prefix(str, cmp) =3D=3D len
> >
> > Here, it's used to make sure we don't have a named mode starting with
> > either e, d, or D.
> >
> > If I understood str_has_prefix() right, str_has_prefix("DUMB-MODE", "D")
> > =3D=3D strlen("DUMB-MODE") would return true, while it's actually what =
we
> > want to avoid.
>=20
> That's not true, str_has_prefix("DUMB-MODE", "D") =3D=3D strlen("D") is.
>=20
> > It's also used indeed in drm_get_tv_mode_from_name(), where we try to
> > match a list of names with one passed as argument.
> >
> > With drm_get_tv_mode_from_name("NSTC", strlen("NTSC")), we would end up
> > calling str_has_prefix("NTSC-J", "NTSC") =3D=3D strlen("NTSC-J") which =
would
> > work. However, we end up calling prefix not a prefix, but an entire
> > string we want to match against, which is very confusing to me too.
>=20
> If I get this right, you have a string and you want to check if that has
> a certain prefix. Additionally, you want to check the prefix is a
> certain length.
>=20
> Sure, that the prefix is a certain length is more of a property of the
> string, which is NUL terminated later than at length, but that's doesn't
> really matter.
>=20
> That condition is simply str_has_prefix(string, prefix) =3D=3D length.

Ack. I'm ok with the implementation being done that way, but I'd really
prefer to still have some macro to make the name less confusing. Would
that work for you? What name would be better in your opinion?

Thanks!
Maxime

--qxovco4c3ldtue3z
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iHUEABYKAB0WIQRcEzekXsqa64kGDp7j7w1vZxhRxQUCYxhY0QAKCRDj7w1vZxhR
xa5XAP9sWfv5tHnxQJpN3hRmX3ErZIQ33tjnxBrnbPX7t8NAjwD/emsxhT5EFzpt
Y/lLUUTztSaE0qu2ofT3EG9ZeOf9fw8=
=dNn8
-----END PGP SIGNATURE-----

--qxovco4c3ldtue3z--