linux-kernel - Re: [PATCH v2 2/2] media: imagination: Add E5010 JPEG Encoder driver

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Thu, 27 Jul 2023 10:19:29 -0400
From:   Nicolas Dufresne <nicolas@...fresne.ca>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
        Devarsh Thakkar <devarsht@...com>, mchehab@...nel.org,
        robh+dt@...nel.org, krzysztof.kozlowski+dt@...aro.org,
        conor+dt@...nel.org, hverkuil-cisco@...all.nl,
        laurent.pinchart@...asonboard.com, eugen.hristev@...labora.com,
        ezequiel@...guardiasur.com.ar, u.kleine-koenig@...gutronix.de,
        sakari.ailus@...ux.intel.com, linux-media@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Cc:     praneeth@...com, nm@...com, vigneshr@...com, a-bhatia1@...com,
        j-luthra@...com, b-brnich@...com, detheridge@...com,
        p-mantena@...com, vijayp@...com
Subject: Re: [PATCH v2 2/2] media: imagination: Add E5010 JPEG Encoder driver

Hi Krzysztof,

Le jeudi 27 juillet 2023 à 14:13 +0200, Krzysztof Kozlowski a écrit :
> On 27/07/2023 13:25, Devarsh Thakkar wrote:
> ...
> 
> > +
> > +static int e5010_release(struct file *file)
> > +{
> > +	struct e5010_dev *dev = video_drvdata(file);
> > +	struct e5010_context *ctx = file->private_data;
> > +
> > +	dprintk(dev, 1, "Releasing instance: 0x%p, m2m_ctx: 0x%p\n", ctx, ctx->fh.m2m_ctx);
> 
> Why do you print pointers? Looks like code is buggy and you still keep
> debugging it.

Its relatively common practice in linux-media to leave a certain level of traces
to help future debugging if a bug is seen. These uses v4l2 debug helper, and are
only going to print if users enable them through the associated sysfs
configuration. I do hope though there isn't any issue with IRQ triggering after
the instance is released, that would be buggy for sure, but I don't think this
is the case considering the level of documented testing that have been done.

I'd be happy to see what others have to say on the subject, as its been a
recurrent subject of confrontation lately. With pretty agressive messages
associated with that.

regards,
Nicolas

p.s. does not invalidate the question, since for this driver, there is only ever
going to be one m2m_ctx, so the question "Why do you print pointers?" is
entirely valid I believe.

. . .