linux-kernel - Re: [PATCH][resend][SCSI] Reduce number of sequential pointer derefs in scsi

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

Message-ID: <AANLkTim1MChiEa9rBL0ZfSCak6G2x4ivK47Svd6swEBS@mail.gmail.com>
Date:	Thu, 18 Nov 2010 13:18:58 -0800
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Jesper Juhl <jj@...osbits.net>
Cc:	linux-scsi@...r.kernel.org,
	"James E.J. Bottomley" <James.Bottomley@...e.de>,
	linux-kernel@...r.kernel.org, Eric Youngdale <eric@...ante.org>,
	"David S. Miller" <davem@...emloft.net>,
	Mike Anderson <andmike@...ibm.com>,
	Russell King <rmk@....linux.org.uk>,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH][resend][SCSI] Reduce number of sequential pointer derefs
 in scsi_error.c and reduce size as well

On Thu, Nov 18, 2010 at 12:49 PM, Jesper Juhl <jj@...osbits.net> wrote:
>
> Fair enough. Seeing your version of this and looking a second time at mine
> I have to agree completely.
> You are absolutely right in stating that the compiler will handle this
> just fine, so it's only a readabillity issue and your version is a *lot*
> more readable than what I came up with.

Btw, one thing to look out for is all those function calls: it really
looks like many of them would be better off not having to dereference
the thing inside each helper function, but just have it dereferenced
in the caller.

You might trying passing in "struct Scsi_Host *host" to a lot of those
helper functions in addition to the 'scmd' part. There's a lot of
"scmd->device->host" going on, and even if you remove some of them
_within_ a function, if you really want to get rid of them you should
probably do one of them in the caller.

That's why the queuecommand() function was changed to take

    struct Scsi_Host *h, struct scsi_cmnd *

as its arguments, because that host is used so commonly. And passing
two arguments is usually free (ie almost all architectures pass it in
registers), and that host variable almost always already exists in the
caller because the caller already needed it.

So if you changed the functions that only take "scsi_cmnd *" as an
argument to match the new queuecommand() interface, I bet you'd get
more cleanups. And the interfaces would match.

              Linus
--
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/