On Tue, 2018-03-27 at 09:36 -0400, Douglas Gilbert wrote:
> On 2018-03-26 08:13 PM, Bart Van Assche wrote:
> > On Sun, 2018-03-18 at 21:59 +0100, Douglas Gilbert wrote:
> > > + /* sense not about current command is termed: deferred */
> >
> > Do we really need comments that explain the SCSI specs? If such a comment is
> > added I think it should be added above the definition of
> > scsi_sense_is_deferred()
> > together with a reference to the "Sense data" section in SPC.
> >
> > > + if (result == 0) {
> > > + /*
> > > + * Unprep the request and put it back at the head of the
> > > + * queue. A new command will be prepared and issued.
> > > + * This block is the same as case ACTION_REPREP in
> > > + * scsi_io_completion_action() above.
> > > */
> > > - if (q->mq_ops) {
> > > + if (q->mq_ops)
> > > scsi_mq_requeue_cmd(cmd);
> > > - } else {
> > > + else {
> > > scsi_release_buffers(cmd);
> > > scsi_requeue_command(q, cmd);
> > > }
> >
> > Have these changes been verified with checkpatch? Checkpatch should have
> > reported
> > the following about the above chunk of code: Unbalanced braces around else
> > statement.
>
> Yes they were, did you check them? If so, with what command line options?
> Since with no options <mkp-4.17/scsi-queue>/scripts/checkpatch.pl returns
> clean for all patches in this set.
If checkpatch did not complain about this patch then I think that that
indicates a bug in checkpatch. The following excerpt from the kernel v4.16-rc7
checkpatch source code shows that checkpatch should complain about the above
changes:
# check for single line unbalanced braces
if ($sline =~ /^.\s*\}\s*else\s*$/ ||
$sline =~ /^.\s*else\s*\{\s*$/) {
CHK("BRACES", "Unbalanced braces around else
statement\n" . $herecurr);
}
Anyway, I think the output of the following commands shows that balancing braces
is the preferred style in the Linux kernel:
$ git grep "$(printf "\t")else {" | wc -l
4971
$ git grep '} else {' | wc -l
61132
Thanks,
Bart.
