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;
Date:   Tue, 16 May 2023 18:12:21 +0200
From:   Joel Granados <j.granados@samsung.com>
To:     Luis Chamberlain <mcgrof@kernel.org>
CC:     Sudip Mukherjee <sudipm.mukherjee@gmail.com>,
        Iurii Zaikin <yzaikin@google.com>,
        Kees Cook <keescook@chromium.org>,
        <linux-kernel@vger.kernel.org>, <linux-fsdevel@vger.kernel.org>
Subject: Re: [PATCH 2/6] parport: Remove register_sysctl_table from
 parport_proc_register
Message-ID: <20230516161221.cojocvsre7uhvz7o@localhost>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg="pgp-sha512";
        protocol="application/pgp-signature"; boundary="xad4rvu4kgoxtnul"
Content-Disposition: inline
In-Reply-To: <20230516143116.hz6rr6kzsviobx35@localhost>
CMS-TYPE: 201P
References: <20230515071446.2277292-1-j.granados@samsung.com>
        <CGME20230515071450eucas1p1625a8639e2b0edf47e41126801d4cbb8@eucas1p1.samsung.com>
        <20230515071446.2277292-3-j.granados@samsung.com>
        <ZGMD4xMRKS8dZJpU@bombadil.infradead.org>
        <20230516143116.hz6rr6kzsviobx35@localhost>
Precedence: bulk

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

On Tue, May 16, 2023 at 04:31:16PM +0200, Joel Granados wrote:
> On Mon, May 15, 2023 at 09:17:39PM -0700, Luis Chamberlain wrote:
> > Awesome!
> >=20
> > On Mon, May 15, 2023 at 09:14:42AM +0200, Joel Granados wrote:
> > > +
> > > +	port_name_len =3D strnlen(port->name, PARPORT_NAME_MAX_LEN);
> > > +	/*
> > > +	 * Allocate a buffer for two paths: dev/parport/PORT and dev/parpor=
t/PORT/devices.
> > > +	 * We calculate for the second as that will give us enough for the =
first.
> > > +	 */
> > > +	tmp_path_len =3D PARPORT_BASE_DEVICES_PATH_SIZE + port_name_len;
> > > +	tmp_dir_path =3D kmalloc(tmp_path_len, GFP_KERNEL);
> >=20
> > Any reason why not kzalloc()?
> nope. Will zero it out.
>=20
> >=20
> > > +	if (!tmp_dir_path) {
> > > +		err =3D -ENOMEM;
> > > +		goto exit_free_t;
> > > +	}
> > > =20
> > > -	t->port_dir[0].procname =3D port->name;
> > > +	if (tmp_path_len
> > > +	    <=3D snprintf(tmp_dir_path, tmp_path_len, "dev/parport/%s/devic=
es", port->name)) {
> >=20
> > Since we are clearing up obfuscation code, it would be nicer to
> > make this easier to read and split the snprintf() into one line, capture
> > the error there. And then in a new line do the check. Even if we have to
> > add a new int value here.
> np. Will do something like this:
>=20
> num_chars_sprinted =3D snprintf(....
> if(tmp_path_len <=3D num_chars_sprinted) {
>   err =3D -ENOENT;
>   ...
> }
>=20
> >=20
> > Other than this I'd just ask to extend the commit log to use
> > the before and after of vmlinux (when this module is compiled in with a=
ll
> > the bells and whistles) with ./scripts/bloat-o-meter.
> >=20
> > Ie build before the patch and cp vmlinux to vmlinux.old and then compare
> > with:
> >=20
> > ./scripts/bloat-o-meter vmlinux.old vmlinux
> >=20
> > Can you also describe any testing if any.
> Sure thing. Will add the bloat-o-meter output on the last patch so as to
> gather the results for all the patches.
>=20
> I'll write some testing info on the patches.
>=20
>=20
> >=20
> > With the above changes, feel free to add to all these patches:
> >=20
> > Reviewed-by: Luis Chamberlain
> Ack
>=20
> >=20
> > > +	if (register_sysctl(tmp_dir_path, t->device_dir) =3D=3D NULL)
> >=20
> > BTW, we should be able to remove now replace register_sysctl_base() wit=
h a simple
> > register_sysctl("kernel", foo), and then one for "fs", and one of "vm"
> > on kernel/sysctl.c and just remove:
> >=20
> >   * DECLARE_SYSCTL_BASE() & register_sysctl_base() & __register_sysctl_=
base()
> >   * and then after all this register_sysctl_table() completely
> >=20
> > Let me know if you'd like a stab at it, or if you prefer me to do that.
> I think I can give it a go. Should I just add that to these set of
> patches? or should we create a new patch set?
I'll send the V2 of this patch set without this. Will add it to the
patch set when I finish if it makes sense. Else I'll just create a new
series.

Best

--=20

Joel Granados

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

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

iQGzBAABCgAdFiEErkcJVyXmMSXOyyeQupfNUreWQU8FAmRjq2MACgkQupfNUreW
QU9rAQv8DIKX8qOK7q31Jp/3mKgPHPSTVWgVhFM51E5QoDUVxfeZRrT6B3lASZLa
eoZDqH14kan8FnkluX4Be/Udtd2s4RIRSSN6MPfpMxqJ4Xu8dwB3oFGLjKznSMKD
ebvfxKtvaIkvwRScs3bET007i8ofSxRBaY+xL//5fH9uXI+qwGUCiv3EnHryXb0Q
A9au6A2CFPVvHvZ8q3VprhBvQh0/iSU5XMgc1+LhOCaffcxNaNA3WuDxZtFteSe6
CPk/J9iKV2qp+M5RHZ8LDU/kpfLqrqY436rC5APYwt0OrcqCcx7APy3IU54LWqTt
dC4SqYkI2I/TuFy8IFCoY2/M4wKKeZnMyC51I15SfpP9m/b8HRxeTInQKHPBqqCx
eFxsIl/ubW/dyiWJyy+sXPH9e00v37ZbamrpBMgpiMYNBYZxitWTuW1BFhOoVZuf
C0VuZeiLiVV6pUZ2hG8YrgKN7sw26S42nsuB8+XroNubW7SQNuxae5t3IzHi1LRL
yM/kS32j
=WpNd
-----END PGP SIGNATURE-----

--xad4rvu4kgoxtnul--