public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
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
Date: Wed, 15 Nov 2023 09:26:10 +0000	[thread overview]
Message-ID: <cc36c3c2f0324ffbab4df089a6cc2621@AcuMS.aculab.com> (raw)
In-Reply-To: <ZVPFUdc6Q/qCI8M7@ideak-desk.fi.intel.com>

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)


      reply	other threads:[~2023-11-15  9:26 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-09 22:55 Build fail in drivers/gpu/drm/i915/display/intel_tc.c David Laight
2023-11-09 23:34 ` Imre Deak
2023-11-10  0:51   ` Linus Torvalds
2023-11-10  9:00     ` David Laight
2023-11-14 19:07       ` Imre Deak
2023-11-15  9:26         ` David Laight [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cc36c3c2f0324ffbab4df089a6cc2621@AcuMS.aculab.com \
    --to=david.laight@aculab.com \
    --cc=imre.deak@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox