All of lore.kernel.org
 help / color / mirror / Atom feed
* Request for mentee selection tasks (Checkpatch Documentation)
@ 2021-07-20  8:43 sri vathsa
  2021-07-20  9:06 ` Dwaipayan Ray
  0 siblings, 1 reply; 5+ messages in thread
From: sri vathsa @ 2021-07-20  8:43 UTC (permalink / raw)
  To: dwaipayanray1, lukas.bulwahn, linux-kernel-mentees


[-- Attachment #1.1: Type: text/plain, Size: 179 bytes --]

Hi,
I'm Srivathsa Dara, I am interested in the Checkpatch Documentation
mentorship program and I would like to work on the tasks for the mentee
selection.

Thanks,
Srivathsa Dara

[-- Attachment #1.2: Type: text/html, Size: 850 bytes --]

[-- Attachment #2: Type: text/plain, Size: 201 bytes --]

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Request for mentee selection tasks (Checkpatch Documentation)
  2021-07-20  8:43 Request for mentee selection tasks (Checkpatch Documentation) sri vathsa
@ 2021-07-20  9:06 ` Dwaipayan Ray
  2021-07-28 16:50   ` sri vathsa
  0 siblings, 1 reply; 5+ messages in thread
From: Dwaipayan Ray @ 2021-07-20  9:06 UTC (permalink / raw)
  To: sri vathsa; +Cc: linux-kernel-mentees

On Tue, Jul 20, 2021 at 2:13 PM sri vathsa <srivathsa729.8@gmail.com> wrote:
>
> Hi,
> I'm Srivathsa Dara, I am interested in the Checkpatch Documentation mentorship program and I would like to work on the tasks for the mentee selection.
>
> Thanks,
> Srivathsa Dara

Thanks for your interest in working with the checkpatch documentation.

The zeroth task is to learn suitable netiquette for the communication with
the kernel community. Below are some basic rules and pointers for this
mentorship. More information on kernel netiquette is also at
https://people.kernel.org/tglx/notes-about-netiquette.

First, please do not top-post.

    A: Because we read from top to bottom, left to right.
    Q: Why should I start my reply below the quoted text?

    A: Because it messes up the order in which people normally read text.
    Q: Why is top-posting such a bad thing?

    A: The lost context.
    Q: What makes top-posted replies harder to read than bottom-posted?

    A: Yes.
    Q: Should I trim down the quoted part of an email to which I'm replying?

Second, please always CC: linux-kernel-mentees@lists.linuxfoundation.org.

Third, set up your email client according to the kernel community
rules. Here is some information to that:

https://www.kernel.org/doc/html/latest/process/submitting-patches.html#no-mime-no-links-no-compression-no-attachments-just-plain-text
https://www.kernel.org/doc/html/latest/process/email-clients.html

Generally more information on submitting patches and responding on
replies is at https://www.kernel.org/doc/html/latest/process/submitting-patches.html

Once the zeroth task is understood, you can now move on to the first
task: Running checkpatch on a specific file.

Get a clone of the Linux kernel repository.
The script checkpatch.pl is under the scripts directory.

Then, the first task is to run checkpatch.pl on a few files below
andshare the results:

drivers/usb/gadget/legacy/raw_gadget.c
drivers/video/fbdev/udlfb.c

Which information on these rules that checkpatch warns about
is available in the Checkpatch Documentation?

If documentation of these rules is available, explain your
understanding of the rules in your own words.
If no information is available in the documentation,
explain your understanding of the rule.

In any case, explain the violation that is checked and raised in the
checkpatch script, i.e., what is implemented in checkpatch to check
the rule and possible violations. Which code in the checkpatch script
is raising the warning? What does it check and how is that
implemented?

Explain how to possibly fix this code with regards to that violation.

Once you succeed on this first task, we inform you about the further
second and third task. If you fail on any of those tasks, you are out
of the selection process.

The selection of the mentee will happen according to schedule,
at earliest on August 12th and at latest at the end of August.
More information is available at
https://docs.linuxfoundation.org/lfx/mentorship/mentorship-program-timelines.

Dwaipayan.
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Request for mentee selection tasks (Checkpatch Documentation)
  2021-07-20  9:06 ` Dwaipayan Ray
@ 2021-07-28 16:50   ` sri vathsa
  2021-08-02 12:23     ` Lukas Bulwahn
  0 siblings, 1 reply; 5+ messages in thread
From: sri vathsa @ 2021-07-28 16:50 UTC (permalink / raw)
  To: Dwaipayan Ray; +Cc: linux-kernel-mentees

On Tue, Jul 20, 2021 at 2:36 PM Dwaipayan Ray <dwaipayanray1@gmail.com> wrote:

> Get a clone of the Linux kernel repository.
I have cloned linux stable repository

> Then, the first task is to run checkpatch.pl on a few files below
> and share the results:
>
> drivers/usb/gadget/legacy/raw_gadget.c

Result:

./drivers/usb/gadget/legacy/raw_gadget.c:52: WARNING: consider using a
completion
./drivers/usb/gadget/legacy/raw_gadget.c:262: WARNING: simple_strtoul
is obsolete, use kstrtoul instead
./drivers/usb/gadget/legacy/raw_gadget.c:701: WARNING: min() should
probably be min_t(unsigned int, io.length, ret)
./drivers/usb/gadget/legacy/raw_gadget.c:1078: WARNING: min() should
probably be min_t(unsigned int, io.length, ret)
total: 0 errors, 4 warnings, 1284 lines checked


> drivers/video/fbdev/udlfb.c

Result:

./drivers/video/fbdev/udlfb.c:82: ERROR: "foo * bar" should be "foo *bar"
./drivers/video/fbdev/udlfb.c:298: WARNING: Block comments should
align the * on each line
./drivers/video/fbdev/udlfb.c:577: WARNING: Missing a blank line after
declarations
./drivers/video/fbdev/udlfb.c:644: WARNING: Missing a blank line after
declarations
./drivers/video/fbdev/udlfb.c:822: WARNING: Missing a blank line after
declarations
./drivers/video/fbdev/udlfb.c:883: WARNING: Missing a blank line after
declarations
./drivers/video/fbdev/udlfb.c:926: WARNING: Prefer 'unsigned int' to
bare use of 'unsigned'
./drivers/video/fbdev/udlfb.c:926: WARNING: Prefer 'unsigned int' to
bare use of 'unsigned'
./drivers/video/fbdev/udlfb.c:926: WARNING: Prefer 'unsigned int' to
bare use of 'unsigned'
./drivers/video/fbdev/udlfb.c:927: WARNING: Prefer 'unsigned int' to
bare use of 'unsigned'
./drivers/video/fbdev/udlfb.c:927: WARNING: Prefer 'unsigned int' to
bare use of 'unsigned'
./drivers/video/fbdev/udlfb.c:1014: WARNING: Missing a blank line
after declarations
./drivers/video/fbdev/udlfb.c:1208: WARNING: Missing a blank line
after declarations
./drivers/video/fbdev/udlfb.c:1233: WARNING: Possible unnecessary 'out
of memory' message
./drivers/video/fbdev/udlfb.c:1257: WARNING: Possible unnecessary 'out
of memory' message
./drivers/video/fbdev/udlfb.c:1425: ERROR: open brace '{' following
function definitions go on the next line
./drivers/video/fbdev/udlfb.c:1429: WARNING: Missing a blank line
after declarations
./drivers/video/fbdev/udlfb.c:1433: ERROR: open brace '{' following
function definitions go on the next line
./drivers/video/fbdev/udlfb.c:1437: WARNING: Missing a blank line
after declarations
./drivers/video/fbdev/udlfb.c:1441: ERROR: open brace '{' following
function definitions go on the next line
./drivers/video/fbdev/udlfb.c:1445: WARNING: Missing a blank line
after declarations
./drivers/video/fbdev/udlfb.c:1449: ERROR: open brace '{' following
function definitions go on the next line
./drivers/video/fbdev/udlfb.c:1453: WARNING: Missing a blank line
after declarations
./drivers/video/fbdev/udlfb.c:1457: ERROR: open brace '{' following
function definitions go on the next line
./drivers/video/fbdev/udlfb.c:1479: ERROR: open brace '{' following
function definitions go on the next line
./drivers/video/fbdev/udlfb.c:1534: WARNING: Symbolic permissions
'S_IWUSR' are not preferred. Consider using octal permissions '0200'.
./drivers/video/fbdev/udlfb.c:1584: WARNING: Comparisons should place
the constant on the right side of the test
./drivers/video/fbdev/udlfb.c:1615: WARNING: Missing a blank line
after declarations
./drivers/video/fbdev/udlfb.c:1658: WARNING: Possible unnecessary 'out
of memory' message
./drivers/video/fbdev/udlfb.c:1888: WARNING: consider using a completion
./drivers/video/fbdev/udlfb.c:1978: WARNING: Symbolic permissions
'S_IWUSR | S_IRUSR | S_IWGRP | S_IRGRP' are not preferred. Consider
using octal permissions '0660'.
./drivers/video/fbdev/udlfb.c:1981: WARNING: Symbolic permissions
'S_IWUSR | S_IRUSR | S_IWGRP | S_IRGRP' are not preferred. Consider
using octal permissions '0660'.
./drivers/video/fbdev/udlfb.c:1984: WARNING: Symbolic permissions
'S_IWUSR | S_IRUSR | S_IWGRP | S_IRGRP' are not preferred. Consider
using octal permissions '0660'.
./drivers/video/fbdev/udlfb.c:1987: WARNING: Symbolic permissions
'S_IWUSR | S_IRUSR | S_IWGRP | S_IRGRP' are not preferred. Consider
using octal permissions '0660'.
./drivers/video/fbdev/udlfb.c:1991: WARNING: quoted string split across lines
./drivers/video/fbdev/udlfb.c:1992: WARNING: quoted string split across lines
total: 7 errors, 29 warnings, 1995 lines checked

> Which information on these rules that checkpatch warns about
> is available in the Checkpatch Documentation?
>
> If documentation of these rules is available, explain your
> understanding of the rules in your own words.
> If no information is available in the documentation,
> explain your understanding of the rule.

> In any case, explain the violation that is checked and raised in the
> checkpatch script, i.e., what is implemented in checkpatch to check
> the rule and possible violations. Which code in the checkpatch script
> is raising the warning? What does it check and how is that
> implemented?
>
> Explain how to possibly fix this code with regards to that violation.
>

---------------------------------------------------------

WARNING: Block comments should align the * on each line
(Documented)
All the '*' in multi-line comments must be aligned correctly to
form a straight line.

checkpatch.pl checks if '*' in multi-line comments
are at same distance from start of line or not.
If not then that means '*' are not aligned.
Following code raises wrong:

if (length($oldindent) ne length($newindent)) {
WARN("BLOCK_COMMENT_STYLE",
     "Block comments should align the * on each line\n" . $hereprev);
}

fix: all '*' are to be aligned to form a vertical
     straight line.

---------------------------------------------------------

ERROR: "foo * bar" should be "foo *bar"
(Documented)
While declaring pointer data-types or functions that return pointer
type data,
'*' must be adjacent to variable name or function name,
not next to data-types.

Wrong usages:
<data-type> * <variable-name>
or
<data-type>* <variable-name>

Correct usage:
<data-type> *<variable-name>

checkpatch.pl raises error if '*'s have spaces between them,
or if '*' have space on both of its sides.
checkpatch.pl also detects which wrong usage mentioned above
is implemented and raises error accordingly.

for this wrong usage:
<data-type> * <variable-name>

error msg will be:
ERROR: "foo * bar" should be "foo *bar"

for this wrong usage:
<data-type>* <variable-name>

error msg will be:
ERROR: "foo* bar" should be "foo *bar"

if ($from ne $to) {
if (ERROR("POINTER_LOCATION",
    "\"(foo$from)\" should be \"(foo$to)\"\n" .  $herecurr) && $fix) {
my $sub_from = $ident;
my $sub_to = $ident;
$sub_to =~ s/\Q$from\E/$to/;
$fixed[$fixlinenr] =~ s@\Q$sub_from\E@$sub_to@;
}
}

if ($from ne $to && $ident !~ /^$Modifier$/) {
if (ERROR("POINTER_LOCATION",
            "\"foo${from}bar\" should be \"foo${to}bar\"\n" .  $herecurr) &&
            $fix) {
my $sub_from = $match;
my $sub_to = $match;
$sub_to =~ s/\Q$from\E/$to/;
$fixed[$fixlinenr] =~ s@\Q$sub_from\E@$sub_to@;
        }
}

fix: '*' must be placed adjacent to variable or function name.

---------------------------------------------------------

WARNING: Missing a blank line after declarations
(Not Documented)
A line must be left blanck after declarations.

checkpatch.pl checks for declaration, if next is also declaration
checkpatch does nothing, instead if there is any other code
checkpatch.pl raises warning.

if (WARN("LINE_SPACING",
         "Missing a blank line after declarations\n" . $hereprev) &&
          $fix) {
fix_insert_line($fixlinenr, "\+");
}

fix: a line must be left blank after declarations.

---------------------------------------------------------

WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
(Not Documented)
'unsigned int' should be used instead of just using 'unsigned'.

checkpatch.pl checks if it is specified unsigned or signed or just
bare data-type, if it is specified only unsigned, it raises warning.

if (WARN("UNSPECIFIED_INT",
         "Prefer '" . trim($sign) . " int" . rtrim($pointer) . "' to
bare use of '$sign" . rtrim($pointer) . "'\n" . $herecurr) &&
         $fix) {
my $decl = trim($sign) . " int ";
my $comp_pointer = $pointer;
$comp_pointer =~ s/\s//g;
$decl .= $comp_pointer;
$decl = rtrim($decl) if ($var eq "");
$fixed[$fixlinenr] =~ s@\b$sign\s*\Q$pointer\E\s*$var\b@$decl$var@;
}

fix: bare 'unsigned' should be replaced with 'unsigned int'

---------------------------------------------------------

WARNING: Possible unnecessary 'out of memory' message
(Not Documented)
Logging messages that show some type of "out of memory" error
are generally unnecessary as there is a generic message and
a stack dump done by the memory subsystem.
These messages generally increase kernel size without much
added value.
So, it's better to remove those messages.

checkpatch.pl checks if any unnecessary OOM messages or logged,
when a variable is compared with NULL in a if statement.
Then it raises warning.

if ($line =~ /^\+.*\b$logFunctions\s*\(/ &&
    $prevline =~ /^[
\+]\s*if\s*\(\s*(\!\s*|NULL\s*==\s*)?($Lval)(\s*==\s*NULL\s*)?\s*\)/
&&
    (defined $1 || defined $3) &&
    $linenr > 3) {
    my $testval = $2;
    my $testline = $lines[$linenr - 3];

    my ($s, $c) = ctx_statement_block($linenr - 3, $realcnt, 0);
#   print("line: <$line>\nprevline: <$prevline>\ns: <$s>\nc: <$c>\n\n\n");

     if ($s =~ /(?:^|\n)[
\+]\s*(?:$Type\s*)?\Q$testval\E\s*=\s*(?:\([^\)]*\)\s*)?\s*$allocFunctions\s*\(/
&&
     $s !~ /\b__GFP_NOWARN\b/ ) {
     WARN("OOM_MESSAGE",
     "Possible unnecessary 'out of memory' message\n" . $hereprev);
     }
}

fix: unnecessary logging oom messages should be avoided.
     In this case dev_err(args..) has to be removed.

---------------------------------------------------------

ERROR: open brace '{' following function definitions go on the next line
(Documentated)
According to kernel coding style while defining functions
opening brace must be on newline, should not be right after function name.

checkpatch.pl checks for open brace violation for many cases at a time,
for all cases except functional block open brace has to on same line, in case
functional blocks open braces have to be on new line.

if ($perl_version_ok &&
    $sline =~ /$Type\s*$Ident\s*$balanced_parens\s*\{/ &&
    $sline !~ /\#\s*define\b.*do\s*\{/ &&
    $sline !~ /}/) {
        if (ERROR("OPEN_BRACE",
                  "open brace '{' following function definitions go on
the next line\n" . $herecurr) &&
                  $fix) {
                  fix_delete_line($fixlinenr, $rawline);
                  my $fixed_line = $rawline;
                  $fixed_line =~ /(^..*$Type\s*$Ident\(.*\)\s*)\{(.*)$/;
                  my $line1 = $1;
                    my $line2 = $2;
                  fix_insert_line($fixlinenr, ltrim($line1));
                  fix_insert_line($fixlinenr, "\+{");
                  if ($line2 !~ /^\s*$/) {
                        fix_insert_line($fixlinenr, "\+\t" . trim($line2));
                       }
         }
}

fix: open brace must be placed in next line of function header line.

---------------------------------------------------------

WARNING: Symbolic permissions 'S_IWUSR' are not preferred. Consider
using octal permissions '0200'.
(Documented)
For permission bits, 4 digit octal permissions (like 0700 or 0444)
should be used, any other base, like decimal should be avoided.

checkpatch.pl checks for uses of S_<PERMS> that could be octal for
readability,
it also gets relevent octal, for S_<PERMS> used, also warns to
consider octal instead.

while ($line =~ m{\b($multi_mode_perms_string_search)\b}g) {
my $oval = $1;
my $octal = perms_to_octal($oval);
if (WARN("SYMBOLIC_PERMS",
  "Symbolic permissions '$oval' are not preferred. Consider using
octal permissions '$octal'.\n" . $herecurr) &&
                    $fix) {
                          $fixed[$fixlinenr] =~ s/\Q$oval\E/$octal/;
}
}

fix: octal permission '0200' has to be used instead of Symbolic permission.

---------------------------------------------------------

WARNING: Comparisons should place the constant on the right side of the test
(Documented)
While comparing with constant value, constant value should be placed
right side of the test.

checkpatch.pl checks for comparisions done with
constants or upper case identifiers on left. If such
compasrison is found then it raises warning.

if ($perl_version_ok &&
    $line =~ /^\+(.*)\b($Constant|[A-Z_][A-Z0-9_]*)\s*($Compare)\s*($LvalOrFunc)/)
{
my $lead = $1;
my $const = $2;
my $comp = $3;
my $to = $4;
my $newcomp = $comp;
if ($lead !~ /(?:$Operators|\.)\s*$/ &&
            $to !~ /^(?:Constant|[A-Z_][A-Z0-9_]*)$/ &&
            WARN("CONSTANT_COMPARISON",
                 "Comparisons should place the constant on the right
side of the test\n" . $herecurr) &&
                 $fix) {
        if ($comp eq "<") {
                                        $newcomp = ">";
                                } elsif ($comp eq "<=") {
                                        $newcomp = ">=";
                                } elsif ($comp eq ">") {
                                        $newcomp = "<";
                                } elsif ($comp eq ">=") {
                                        $newcomp = "<=";
                                }
                                $fixed[$fixlinenr] =~
s/\(\s*\Q$const\E\s*$Compare\s*\Q$to\E\s*\)/($to $newcomp $const)/;
                        }
}

fix: constant value is to placed right side of test.

---------------------------------------------------------

WARNING: consider using a completion
(Not Documented)
This warning suggests to use completion instead of semaphore.

checkpatch.pl checks for iniialized semaphores, by checking
for sema_init, if semaphores is initialized warns to use completion
instead of semaphore.

if ($line =~ /^.\s*sema_init.+,\W?0\W?\)/) {
       WARN("CONSIDER_COMPLETION",
            "consider using a completion\n" . $herecurr);
}

fix: completion has to be used instead of semaphore.

---------------------------------------------------------

WARNING: quoted string split across lines
(Documented)
According to kernel coding style, user visible strings can't be splited.
Because that breaks the ability to grep for them.

checkpatch.pl checks Check for user-visible strings broken across lines,
which breaks the ability to grep for the string. If previous string ends
with '\t', '\n', '\r', ';', or '{' (common in inline assembly) or
is a octal \123 or hexadecimal \xaf value they are not considered as
quoted string splits.

if ($line =~ /^\+\s*$String/ &&
$prevline =~ /"\s*$/ &&
$prevrawline !~
/(?:\\(?:[ntr]|[0-7]{1,3}|x[0-9a-fA-F]{1,2})|;\s*|\{\s*)"\s*$/) {
if (WARN("SPLIT_STRING",
         "quoted string split across lines\n" . $hereprev) &&
                         $fix &&
                         $prevrawline =~ /^\+.*"\s*$/ &&
                         $last_coalesced_string_linenr != $linenr - 1) {
                                my $extracted_string =
get_quoted_string($line, $rawline);
                                my $comma_close = "";
                                if ($rawline =~
/\Q$extracted_string\E(\s*\)\s*;\s*$|\s*,\s*)/) {
                                        $comma_close = $1;
                                }

                                fix_delete_line($fixlinenr - 1, $prevrawline);
                                fix_delete_line($fixlinenr, $rawline);
                                my $fixedline = $prevrawline;
                                $fixedline =~ s/"\s*$//;
                                $fixedline .=
substr($extracted_string, 1) . trim($comma_close);
                                fix_insert_line($fixlinenr - 1, $fixedline);
                                $fixedline = $rawline;
                                $fixedline =~
s/\Q$extracted_string\E\Q$comma_close\E//;
                                if ($fixedline !~ /\+\s*$/) {
                                        fix_insert_line($fixlinenr, $fixedline);
                                }
                                $last_coalesced_string_linenr = $linenr;
}
}

fix: user-readle strings must not be splited or '\n', '\t'
are to be used to supress warning.

---------------------------------------------------------

WARNING: simple_strtoul is obsolete, use kstrtoul instead
(Documented)
Usage of simple_strtol(), simple_strtoll(), simple_strtoul(),
and simple_strtoull() is not encouraged because these functions
explicitly ignore overflows, which may lead to unexpected results
in callers.
kstrtol(), kstrtoll(), kstrtoul(), and kstrtoull() functions
respectively tend to be the correct replacements.

checkpatch.pl recommends kstrto* over simple_strto* and strict_strto*,
it checks if simple_strto* or strict_strto* is used, and warns to use
corresponding kstrto*.

if ($line =~ /\b((simple|strict)_(strto(l|ll|ul|ull)))\s*\(/) {
        WARN("CONSIDER_KSTRTO",
             "$1 is obsolete, use k$3 instead\n" . $herecurr);
}

fix: kstrtoul has to be used insted of simple_strtoul

---------------------------------------------------------

WARNING: min() should probably be min_t(unsigned int, io.length, ret)
(Not Documented)
min_t() is prefered instead of using min() in this particular case,
because min_t() defined to return a minimum value of specified_type,
whereas min() return minimum value of same or related data-type.

checkpatch.pl checks if any min(), max() used while typecasting,
if used warns to used min_t(), max_t() respectively.

if ($perl_version_ok &&
    defined $stat &&
    $stat =~ /^\+(?:.*?)\b(min|max)\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\)/) {
        if (defined $2 || defined $7) {
             my $call = $1;
             my $cast1 = deparenthesize($2);
             my $arg1 = $3;
             my $cast2 = deparenthesize($7);
             my $arg2 = $8;
             my $cast;

             if ($cast1 ne "" && $cast2 ne "" && $cast1 ne $cast2) {
                       $cast = "$cast1 or $cast2";
              } elsif ($cast1 ne "") {
                       $cast = $cast1;
              } else {
                       $cast = $cast2;
              }
                       WARN("MINMAX",
                            "$call() should probably be
${call}_t($cast, $arg1, $arg2)\n" . "$here\n$stat\n");
              }
}

fix: min_t() has to be instead of min()

---------------------------------------------------------

> Once you succeed on this first task, we inform you about the further
> second and third task. If you fail on any of those tasks, you are out
> of the selection process.
>
> The selection of the mentee will happen according to schedule,
> at earliest on August 12th and at latest at the end of August.
> More information is available at
> https://docs.linuxfoundation.org/lfx/mentorship/mentorship-program-timelines.
>
> Dwaipayan.
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Request for mentee selection tasks (Checkpatch Documentation)
  2021-07-28 16:50   ` sri vathsa
@ 2021-08-02 12:23     ` Lukas Bulwahn
  0 siblings, 0 replies; 5+ messages in thread
From: Lukas Bulwahn @ 2021-08-02 12:23 UTC (permalink / raw)
  To: sri vathsa; +Cc: Dwaipayan Ray, linux-kernel-mentees

Dear Sri,

> > Then, the first task is to run checkpatch.pl on a few files below
> > and share the results:
> >
> > drivers/usb/gadget/legacy/raw_gadget.c
>
> Result:
>
> ./drivers/usb/gadget/legacy/raw_gadget.c:52: WARNING: consider using a
> completion

(snip)

>
> WARNING: consider using a completion
> (Not Documented)
> This warning suggests to use completion instead of semaphore.
>
> checkpatch.pl checks for iniialized semaphores, by checking
> for sema_init, if semaphores is initialized warns to use completion
> instead of semaphore.
>
> if ($line =~ /^.\s*sema_init.+,\W?0\W?\)/) {
>        WARN("CONSIDER_COMPLETION",
>             "consider using a completion\n" . $herecurr);
> }
>
> fix: completion has to be used instead of semaphore.
>

Can you do some further investigation and research what this warning is about?

E.g., Look at the commit and its commit message that introduced this
rule in checkpatch. Look in the kernel documentation for relevant
information on semaphores and completions, the difference and the
reasons to use one or the other synchonisation mechanism.

Summarize this information and provide some documentation on this
warning with various pointers for further reading and share that with
us.

We found 16 cases in the kernel tree:

drivers/crypto/ux500/cryp/cryp_core.c:1593:
WARNING:CONSIDER_COMPLETION: consider using a completion
drivers/crypto/ux500/hash/hash_core.c:1946:
WARNING:CONSIDER_COMPLETION: consider using a completion
drivers/gpu/drm/udl/udl_main.c:213: WARNING:CONSIDER_COMPLETION:
consider using a completion
drivers/infiniband/ulp/isert/ib_isert.c:2275:
WARNING:CONSIDER_COMPLETION: consider using a completion
drivers/input/misc/hp_sdc_rtc.c:88: WARNING:CONSIDER_COMPLETION:
consider using a completion
drivers/input/serio/hil_mlc.c:934: WARNING:CONSIDER_COMPLETION:
consider using a completion
drivers/input/serio/hp_sdc.c:904: WARNING:CONSIDER_COMPLETION:
consider using a completion
drivers/input/serio/hp_sdc.c:1033: WARNING:CONSIDER_COMPLETION:
consider using a completion
drivers/md/dm-region-hash.c:217: WARNING:CONSIDER_COMPLETION: consider
using a completion
drivers/parport/share.c:465: WARNING:CONSIDER_COMPLETION: consider
using a completion
drivers/usb/gadget/legacy/raw_gadget.c:52:
WARNING:CONSIDER_COMPLETION: consider using a completion
drivers/video/fbdev/udlfb.c:1888: WARNING:CONSIDER_COMPLETION:
consider using a completion
fs/xfs/xfs_buf.c:237: WARNING:CONSIDER_COMPLETION: consider using a completion
include/drm/task_barrier.h:59: WARNING:CONSIDER_COMPLETION: consider
using a completion
include/drm/task_barrier.h:60: WARNING:CONSIDER_COMPLETION: consider
using a completion
kernel/seccomp.c:1769: WARNING:CONSIDER_COMPLETION: consider using a completion

Can you please check which of these cases should be refactored (so the
checkpatch warning points to a valid suggestion to refactor the code)
or if checkpatch is wrong with its suggestion and why?

Thanks,

Lukas
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Request for mentee selection tasks (Checkpatch Documentation)
@ 2021-07-29  2:08 Amit Kumar Yadav
  0 siblings, 0 replies; 5+ messages in thread
From: Amit Kumar Yadav @ 2021-07-29  2:08 UTC (permalink / raw)
  To: dwaipayanray1, lukas.bulwahn, linux-kernel-mentees

I am interested in the Checkpatch Documentation mentorship program and
I would like to work on the tasks for the mentee selection.
Thanks.
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-08-02 12:21 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-20  8:43 Request for mentee selection tasks (Checkpatch Documentation) sri vathsa
2021-07-20  9:06 ` Dwaipayan Ray
2021-07-28 16:50   ` sri vathsa
2021-08-02 12:23     ` Lukas Bulwahn
2021-07-29  2:08 Amit Kumar Yadav

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.