Docs: Remove "tips and tricks" from SubmittingPatches
This section was just a weird collection of stuff that is better found elsewhere. The "coding style" section somewhat duplicated the previous coding style section; the useful information there has been collected into a single place. Signed-off-by: Jonathan Corbet <corbet@lwn.net>
This commit is contained in:
Родитель
97bf6af1f9
Коммит
6de16eba62
|
@ -193,17 +193,33 @@ then only post say 15 or so at a time and wait for review and integration.
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
4) Style check your changes.
|
4) Style-check your changes.
|
||||||
|
----------------------------
|
||||||
|
|
||||||
Check your patch for basic style violations, details of which can be
|
Check your patch for basic style violations, details of which can be
|
||||||
found in Documentation/CodingStyle. Failure to do so simply wastes
|
found in Documentation/CodingStyle. Failure to do so simply wastes
|
||||||
the reviewers time and will get your patch rejected, probably
|
the reviewers time and will get your patch rejected, probably
|
||||||
without even being read.
|
without even being read.
|
||||||
|
|
||||||
At a minimum you should check your patches with the patch style
|
One significant exception is when moving code from one file to
|
||||||
checker prior to submission (scripts/checkpatch.pl). You should
|
another -- in this case you should not modify the moved code at all in
|
||||||
be able to justify all violations that remain in your patch.
|
the same patch which moves it. This clearly delineates the act of
|
||||||
|
moving the code and your changes. This greatly aids review of the
|
||||||
|
actual differences and allows tools to better track the history of
|
||||||
|
the code itself.
|
||||||
|
|
||||||
|
Check your patches with the patch style checker prior to submission
|
||||||
|
(scripts/checkpatch.pl). Note, though, that the style checker should be
|
||||||
|
viewed as a guide, not as a replacement for human judgment. If your code
|
||||||
|
looks better with a violation then its probably best left alone.
|
||||||
|
|
||||||
|
The checker reports at three levels:
|
||||||
|
- ERROR: things that are very likely to be wrong
|
||||||
|
- WARNING: things requiring careful review
|
||||||
|
- CHECK: things requiring thought
|
||||||
|
|
||||||
|
You should be able to justify all violations that remain in your
|
||||||
|
patch.
|
||||||
|
|
||||||
|
|
||||||
5) Select e-mail destination.
|
5) Select e-mail destination.
|
||||||
|
@ -684,100 +700,9 @@ new/deleted or renamed files.
|
||||||
With rename detection, the statistics are rather different [...]
|
With rename detection, the statistics are rather different [...]
|
||||||
because git will notice that a fair number of the changes are renames.
|
because git will notice that a fair number of the changes are renames.
|
||||||
|
|
||||||
-----------------------------------
|
|
||||||
SECTION 2 - HINTS, TIPS, AND TRICKS
|
|
||||||
-----------------------------------
|
|
||||||
|
|
||||||
This section lists many of the common "rules" associated with code
|
|
||||||
submitted to the kernel. There are always exceptions... but you must
|
|
||||||
have a really good reason for doing so. You could probably call this
|
|
||||||
section Linus Computer Science 101.
|
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
1) Read Documentation/CodingStyle
|
|
||||||
|
|
||||||
Nuff said. If your code deviates too much from this, it is likely
|
|
||||||
to be rejected without further review, and without comment.
|
|
||||||
|
|
||||||
One significant exception is when moving code from one file to
|
|
||||||
another -- in this case you should not modify the moved code at all in
|
|
||||||
the same patch which moves it. This clearly delineates the act of
|
|
||||||
moving the code and your changes. This greatly aids review of the
|
|
||||||
actual differences and allows tools to better track the history of
|
|
||||||
the code itself.
|
|
||||||
|
|
||||||
Check your patches with the patch style checker prior to submission
|
|
||||||
(scripts/checkpatch.pl). The style checker should be viewed as
|
|
||||||
a guide not as the final word. If your code looks better with
|
|
||||||
a violation then its probably best left alone.
|
|
||||||
|
|
||||||
The checker reports at three levels:
|
|
||||||
- ERROR: things that are very likely to be wrong
|
|
||||||
- WARNING: things requiring careful review
|
|
||||||
- CHECK: things requiring thought
|
|
||||||
|
|
||||||
You should be able to justify all violations that remain in your
|
|
||||||
patch.
|
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
2) #ifdefs are ugly
|
|
||||||
|
|
||||||
Code cluttered with ifdefs is difficult to read and maintain. Don't do
|
|
||||||
it. Instead, put your ifdefs in a header, and conditionally define
|
|
||||||
'static inline' functions, or macros, which are used in the code.
|
|
||||||
Let the compiler optimize away the "no-op" case.
|
|
||||||
|
|
||||||
Simple example, of poor code:
|
|
||||||
|
|
||||||
dev = alloc_etherdev (sizeof(struct funky_private));
|
|
||||||
if (!dev)
|
|
||||||
return -ENODEV;
|
|
||||||
#ifdef CONFIG_NET_FUNKINESS
|
|
||||||
init_funky_net(dev);
|
|
||||||
#endif
|
|
||||||
|
|
||||||
Cleaned-up example:
|
|
||||||
|
|
||||||
(in header)
|
|
||||||
#ifndef CONFIG_NET_FUNKINESS
|
|
||||||
static inline void init_funky_net (struct net_device *d) {}
|
|
||||||
#endif
|
|
||||||
|
|
||||||
(in the code itself)
|
|
||||||
dev = alloc_etherdev (sizeof(struct funky_private));
|
|
||||||
if (!dev)
|
|
||||||
return -ENODEV;
|
|
||||||
init_funky_net(dev);
|
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
3) 'static inline' is better than a macro
|
|
||||||
|
|
||||||
Static inline functions are greatly preferred over macros.
|
|
||||||
They provide type safety, have no length limitations, no formatting
|
|
||||||
limitations, and under gcc they are as cheap as macros.
|
|
||||||
|
|
||||||
Macros should only be used for cases where a static inline is clearly
|
|
||||||
suboptimal [there are a few, isolated cases of this in fast paths],
|
|
||||||
or where it is impossible to use a static inline function [such as
|
|
||||||
string-izing].
|
|
||||||
|
|
||||||
'static inline' is preferred over 'static __inline__', 'extern inline',
|
|
||||||
and 'extern __inline__'.
|
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
4) Don't over-design.
|
|
||||||
|
|
||||||
Don't try to anticipate nebulous future cases which may or may not
|
|
||||||
be useful: "Make it as simple as you can, and no simpler."
|
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
----------------------
|
----------------------
|
||||||
SECTION 3 - REFERENCES
|
SECTION 2 - REFERENCES
|
||||||
----------------------
|
----------------------
|
||||||
|
|
||||||
Andrew Morton, "The perfect patch" (tpp).
|
Andrew Morton, "The perfect patch" (tpp).
|
||||||
|
|
Загрузка…
Ссылка в новой задаче