Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.35 as permitted sender) client-ip=23.128.96.35;
From:   David Laight <David.Laight@ACULAB.COM>
To:     "'imre.deak@intel.com'" <imre.deak@intel.com>,
        Linus Torvalds <torvalds@linux-foundation.org>
CC:     "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: Build fail in drivers/gpu/drm/i915/display/intel_tc.c
Thread-Topic: Build fail in drivers/gpu/drm/i915/display/intel_tc.c
Thread-Index: AdoTX9s2sPRYSwRmSkOwOZ8JicXQjgAEEIUiABCaxIAA3tRGgAAdyf/A
Date:   Wed, 15 Nov 2023 09:26:10 +0000
Message-ID: <cc36c3c2f0324ffbab4df089a6cc2621@AcuMS.aculab.com>
References: <4df68c74c4da46ac943ac21578e9b084@AcuMS.aculab.com>
 <ZU1shQH64ryxp/l5@ideak-desk.fi.intel.com>
 <CAHk-=wgiPxF6GnvaSGV+xOjcBO-T1vxLYhSvho0Q5Cdqw7t74w@mail.gmail.com>
 <39709b4bb3e447159759a9923bd4a564@AcuMS.aculab.com>
 <ZVPFUdc6Q/qCI8M7@ideak-desk.fi.intel.com>
In-Reply-To: <ZVPFUdc6Q/qCI8M7@ideak-desk.fi.intel.com>
Accept-Language: en-GB, en-US
MIME-Version: 1.0
Content-Language: en-US
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8BIT
Precedence: bulk

From: Imre Deak
> Sent: 14 November 2023 19:08
> 
> On Fri, Nov 10, 2023 at 09:00:21AM +0000, David Laight wrote:
> > From: Linus Torvalds
> > > Sent: 10 November 2023 00:52
> > >
> > > On Thu, 9 Nov 2023 at 15:34, Imre Deak <imre.deak@intel.com> wrote:
> > > >
> > > > The compiler warn should be fixed/suppressed by:
> > > > https://lore.kernel.org/all/20231026125636.5080-1-nirmoy.das@intel.com
> > >
> > > Ugh, so now it's a dynamic allocation, wasting memory, and a pointer
> > > to it, using as much memory as the array did in the first place.
> > >
> > > All because of a pointless warning that was a false positive - and was
> > > always harmless anyway, since snprintf() is safe (ie it was only a
> > > "might be truncated").
> >
> > That entire warning for snprintf() is a false positive.
> > The ones that are likely to overflow unexpectedly are the ones
> > with a "%s" format for a 'char *' pointer where there is no
> > implied length.
> >
> > The same check for printf() using the implied buffer length
> > probably does make sense.
> >
> > I don't even think there is a way of avoiding it on a case by case
> > basis - apart from passing both the buffer address and length
> > to an inline asm that the compiler has to assume might change
> > the values, but that tends to generate an extra 'mov' instruction
> > for no good reason at all.
> >
> > >
> > > Please don't do this. Either do that ((tc_port & 7) + 1) suggestion of
> > > David's, or just do '%c' and make the expression be
> > >
> > >   '1' + tc_port
> > >
> > > which should be fine since I915_MAX_PORTS is 8 or whatever.
> 
> Ok, the patch above was merged already to drm-tip,

I'd noticed ....

> but I agree not to use kasprintf for this.
> The above looks ok and there is already
> tc_port_name() for this, would just need to export that from
> intel_display.h.

I'm not sure that #define does anything except obscure what
is going on.
Anyone reading the code is going to search for it and then sigh.
But it is your code :-)

> I can follow up with a patch for this, or if you or David wanted to do
> that please send a patch to the intel-gfx@lists.freedesktop.org list.

Probably much easier for you to do it.
My process to get patches emailed is a PITA.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)