On 7/6/22, Akira Yokosawa <akiyks@xxxxxxxxx> wrote: > Hi, > Minor nit on reST syntax. > > On Mon, 4 Jul 2022 19:57:57 -0300, Martin Fernandez wrote: >> Add a description, an example and a possible workaround to the >> MACRO_ARG_REUSE check. >> >> Signed-off-by: Martin Fernandez <martin.fernandez@xxxxxxxxxxxxx> >> Acked-by: Dwaipayan Ray <dwaipayanray1@xxxxxxxxx> >> --- >> Documentation/dev-tools/checkpatch.rst | 20 ++++++++++++++++++++ >> 1 file changed, 20 insertions(+) >> >> diff --git a/Documentation/dev-tools/checkpatch.rst >> b/Documentation/dev-tools/checkpatch.rst >> index b52452bc2963..86545c65cf7b 100644 >> --- a/Documentation/dev-tools/checkpatch.rst >> +++ b/Documentation/dev-tools/checkpatch.rst >> @@ -759,6 +759,26 @@ Indentation and Line Breaks >> Macros, Attributes and Symbols >> ------------------------------ >> >> + **ARG_REUSE**> + Using the same argument multiple times in the macro >> definition >> + would lead to unwanted side-effects. > > You don't need manual emphasis as above, as this list is already > in the form of so-called "Definition Lists" [1, 2]. > > [1]: > https://docutils.sourceforge.io/docs/ref/rst/restructuredtext.html#definition-lists > [2]: > https://www.sphinx-doc.org/en/master/usage/restructuredtext/basics.html#lists-and-quote-like-blocks Thank you for the references. > Defined terms will be automatically emphasized by Sphinx and > should look in bold face in the generated HTML/PDF. > (Style of emphasis might be customized by configuration.) > > It looks like there exists other similar patterns in this file > (or might as well be in other related .rst files). I'd suggest > removing those manual emphases in a follow-up patch. > > This is only a weak suggestion, and there is no urgency. > Of course, if you have a reason to do the manual emphases, > there is no need to change. That's interesting. Didn't really know that. I just saw this unknown warning for me and since there were no documentation about it I decided to quickly add it using the rest of the document as a template. I agree that that's not a very good approach but it was very quick :) I'll consider checking the syntax of the whole document for further patches, thank you for the suggestion. > Thanks, Akira > >> + >> + For example, given a `min` macro defined like:: >> + >> + #define min(x, y) ((x) < (y) ? (x) : (y)) >> + >> + If you call it with `min(foo(x), 0)`, it would expand to:: >> + >> + foo(x) < 0 ? foo(x) : 0 >> + >> + If `foo` has side-effects or it's an expensive calculation the >> + results might not be what the user intended. >> + >> + For a workaround the idea is to define local variables to hold the >> + macro's arguments. Checkout the actual implementation of `min` in >> + include/linux/minmax.h for the full implementation of the >> + workaround. >> + >> **ARRAY_SIZE** >> The ARRAY_SIZE(foo) macro should be preferred over >> sizeof(foo)/sizeof(foo[0]) for finding number of elements in an >> -- >> 2.30.2 >
