linux-kernel - Re: [PATCH] i.MX8 ISI crossbar: simplify a couple of error messages

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20250521102908.GD12514@pendragon.ideasonboard.com>
Date: Wed, 21 May 2025 12:29:08 +0200
From: Laurent Pinchart <laurent.pinchart@...asonboard.com>
To: Krzysztof Hałasa <khalasa@...p.pl>
Cc: Mauro Carvalho Chehab <mchehab@...nel.org>,
	Shawn Guo <shawnguo@...nel.org>,
	Sascha Hauer <s.hauer@...gutronix.de>,
	Pengutronix Kernel Team <kernel@...gutronix.de>,
	Fabio Estevam <festevam@...il.com>, linux-media@...r.kernel.org,
	imx@...ts.linux.dev, linux-arm-kernel@...ts.infradead.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] i.MX8 ISI crossbar: simplify a couple of error messages

Hi Krzysztof,

On Tue, May 20, 2025 at 01:19:12PM +0200, Krzysztof Hałasa wrote:
> Laurent Pinchart <laurent.pinchart@...asonboard.com> writes:
> 
> > The goal was indeed to save memory.
> >
> >> --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
> >> +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
> >> @@ -352,9 +352,8 @@ static int mxc_isi_crossbar_enable_streams(struct v4l2_subdev *sd,
> >>                                                sink_streams);
> >>               if (ret) {
> >>                       dev_err(xbar->isi->dev,
> >> -                             "failed to %s streams 0x%llx on '%s':%u: %d\n",
> >> -                             "enable", sink_streams, remote_sd->name,
> >> -                             remote_pad, ret);
> >> +                             "failed to enable streams 0x%llx on '%s':%u: %d\n",
> >> +                             sink_streams, remote_sd->name, remote_pad, ret);
> >>                       mxc_isi_crossbar_gasket_disable(xbar, sink_pad);
> >>                       return ret;
> >>               }
> >> @@ -392,9 +391,8 @@ static int mxc_isi_crossbar_disable_streams(struct v4l2_subdev *sd,
> >>                                                 sink_streams);
> >>               if (ret)
> >>                       dev_err(xbar->isi->dev,
> >> -                             "failed to %s streams 0x%llx on '%s':%u: %d\n",
> >> -                             "disable", sink_streams, remote_sd->name,
> >> -                             remote_pad, ret);
> >> +                             "failed to disable streams 0x%llx on '%s':%u: %d\n",
> >> +                             sink_streams, remote_sd->name, remote_pad, ret);
> 
> It appears the current code saves (in my default build) 8 bytes of
> memory, at the cost of readability and inability to search with grep:
> 
> Current:
> Name          Size      File off
> .text         00000bf4  00000040
> .data         00000000  00000c34
> .rodata.str1.8 000001b8 00000c38
> __jump_table  00000030  00000df0
> .rodata       000001b8  00000e20
> 
> With patch:
> Name          Size      File off
> .text         00000bd4  00000040
> .data         00000000  00000c14
> .rodata.str1.8 000001e0 00000c18
> __jump_table  00000030  00000df8
> .rodata       000001b8  00000e28
> 
> Built as a module it doesn't even save that - the number of pages stays
> the same.
> 
> Just FYI.

Fair enough. I'll take the patch, with an update to the commit message:

media: nxp: imx8-isi: Simplify a couple of error messages

The error messages in the mxc_isi_crossbar_enable_streams() and 
mxc_isi_crossbar_disable_streams() functions are similar, with a single
word difference between them. The word is moved out of the format string
to a separate dev_err() argument to try and save memory through string 
de-duplication. The total savings are however small, as the .data size 
reduction is partly offset by a .text size increase, with a total saving
of 8 bytes in total on an ARM64 platforms. They also come at the cost of
making the error message difficult to grep, which outweights the gains.

Simplify the error messages to make them grep-able.

Reviewed-by: Laurent Pinchart <laurent.pinchart@...asonboard.com>

-- 
Regards,

Laurent Pinchart