public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Dominique Martinet <asmadeus@codewreck.org>
To: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>,
	Joonas Lahtinen <joonas.lahtinen@linux.intel.com>,
	Rodrigo Vivi <rodrigo.vivi@intel.com>,
	David Airlie <airlied@linux.ie>,
	intel-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org,
	dri-devel@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH] i915/intel_tv_get_modes: fix strncpy truncation warning
Date: Thu, 12 Jul 2018 15:55:26 +0200	[thread overview]
Message-ID: <20180712135526.GA5463@nautica> (raw)
In-Reply-To: <20180712124401.GZ5565@intel.com>

Ville Syrjälä wrote on Thu, Jul 12, 2018:
> On Wed, Jul 11, 2018 at 09:46:15AM +0200, Dominique Martinet wrote:
> > This is effectively no-op as the next line writes a nul at the final
> 
> What is "This". Please write self contained commit messages.

This could either be 'this commit' as a whole or if you look only at the
commit message 'this strncpy fix' from the title (which is arguably the
same), and both interpretations sound fairly understandable in the
context of the title line without seeing the patch to me... Although
I'll admit this is difficult to judge of that as the author.

Thanksfully, the v2 of the patch didn't use this wording but while I
agree the message could be better I do not think it was horrible.


> > drivers/gpu/drm/i915/intel_tv.c: In function ‘intel_tv_get_modes’:
> > drivers/gpu/drm/i915/intel_tv.c:1358:3: error: ‘strncpy’ specified bound 32 equals destination size [-Werror=stringop-truncation]
> >    strncpy(mode_ptr->name, input->name, DRM_DISPLAY_MODE_LEN);
> >    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > cc1: all warnings being treated as errors
> 
> That warning should be in the actual commit message.

Yes and no, I gave it for referrence but when you update to gcc 8 you
will literally see it all over the place.
The words "strncpy truncation warning" is really precise once you've
seen them a few times and there are litteraly hundred of these warnings
in the kernel, some have already been fixed taking a glance at the git
log, some with and without the warning message.
I don't think it's worth polluting the git log with this many
warnings... Which leads to...

> This same pattern is used all over drm. Can you go and fix them all up?
> One might even consider writing a cocci patch for it ;)

Now this is something I can agree with.
This patch really was just a stop-gap measure because I could not build
the kernel at all without it, but yes I did consider having a look at
others.

Unfortunately coccinelle does not run on fedora 28 (and doesn't look
like it will fix itself any time soon, there is a bug report[1] open
since February that didn't get much love lately - I was just looking at
it a few days ago)

I think in this case it might actually be faster to look at gcc warnings
and s/strncpy/strlcpy/, but I am curious about Coccinelle so this is a
good excuse to look at it, I'll report back in a bit after poking at
that bug report and figuring out how coccinelle works.

I do not guarantee speed however, if anyone sees this and feels put off
from donig it themselves, please go ahead and just drop me a word.


[1] https://bugzilla.redhat.com/show_bug.cgi?id=1544204

Thanks, and sorry for the mail longer than I originally intended,
-- 
Dominique Martinet

  reply	other threads:[~2018-07-12 13:55 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-11  7:46 [PATCH] i915/intel_tv_get_modes: fix strncpy truncation warning Dominique Martinet
2018-07-11 14:48 ` Chris Wilson
2018-07-11 23:24 ` [PATCH v2] " Dominique Martinet
2018-07-12  7:39   ` Chris Wilson
2018-07-12  9:19   ` Chris Wilson
2018-07-12 12:44 ` [Intel-gfx] [PATCH] " Ville Syrjälä
2018-07-12 13:55   ` Dominique Martinet [this message]
2018-07-12 14:10     ` Ville Syrjälä
2018-07-12 14:30       ` Dominique Martinet

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=20180712135526.GA5463@nautica \
    --to=asmadeus@codewreck.org \
    --cc=airlied@linux.ie \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=jani.nikula@linux.intel.com \
    --cc=joonas.lahtinen@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rodrigo.vivi@intel.com \
    --cc=ville.syrjala@linux.intel.com \
    /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