linux-kernel - Re: [PATCH] staging: usbip: stub_main.c: Cleaning up missing null-terminate after strncpy call

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] [day] [month] [year] [list]

Message-ID: <20140612213916.GX5500@mwanda>
Date:	Fri, 13 Jun 2014 00:39:16 +0300
From:	Dan Carpenter <dan.carpenter@...cle.com>
To:	Rickard Strandqvist <rickard_strandqvist@...ctrumdigital.se>
Cc:	devel@...verdev.osuosl.org,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	linux-usb@...r.kernel.org, navin patidar <navinp@...c.in>,
	Valentina Manea <valentina.manea.m@...il.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] staging: usbip: stub_main.c: Cleaning up missing
 null-terminate after strncpy call

On Thu, Jun 12, 2014 at 11:09:20PM +0200, Rickard Strandqvist wrote:
> I agree that you should not do patches just to silence a static control program.
> 
> Concerning (len >= BUSID_SIZE) I agree! But I usually try to change as
> little as possible in the patches I do. But perhaps I should not think
> that way.

The "one thing per patch" rule is tricky for a lot of people when they
start upstream kernel programming.

1) If you are going to need to backport a patch then write the simplest
   version you can and do any cleanup in a later patch.

2) Most of these patches will not need to back ported.  The "one thing"
is about how you describe the patch.  You're allowed to make minor
closely related changes.  In this case the one thing would be, "Clean up
string handling in xxx()".  The commit message would say:

cppcheck has a false positive here.  I looked at the code and it's ok,
but a bit messy.  I have cleaned it up by doing:

	1) use strclpy() or whatever
	2) cleanup a condition
	3) remove obvious comments
	4) blah blah blah

The new code is simpler and doesn't generate a cppcheck warning.

Where checkpatch.pl get into trouble is that they say "I am doing one
thing and it is to fix everything in filename.c".  That's everything and
not "one thing".

regards,
dan carpenter

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/