linux-kernel - Re: [GIT PULL] SCSI queuecommand API change for 2.6.37-rc1

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]

Date:	Fri, 12 Nov 2010 18:30:25 -0800
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Jeff Garzik <jeff@...zik.org>
Cc:	James Bottomley <James.Bottomley@...e.de>,
	"Nicholas A. Bellinger" <nab@...ux-iscsi.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	linux-scsi <linux-scsi@...r.kernel.org>,
	linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [GIT PULL] SCSI queuecommand API change for 2.6.37-rc1

On Fri, Nov 12, 2010 at 6:09 PM, Jeff Garzik <jeff@...zik.org> wrote:
>
> For the record, I prefer a rename to a prototype change.

I'm certainly ok with a rename in most cases, but in this case I think
the prototype change might be better:

 - adding something like a "lck" prefix (or postfix) to the function
pointer name is the logical thing to do, but it's only logical right
_now_. Once it all becomes historic and the normal way to do things,
rather than a flag-day issue, any name change is likely to just be
annoying.

   So then we' either have to plan to rename it back once all the
flag-day issues are over and everybody is convinced that things have
been converted both in and out of tree, or we'd have to live with an
odd "lock" thing. We've seen this issue before: now we've finally
gotten rid of the BKL, and what do we call our "ioctl()" function? We
call it "unlocked_ioctl". It still makes sense today because the BKL
is in fresh memory, but I already wince when I look at it.

 - it would add another line of change to each driver, just for the
name change. Maybe not a big deal, and maybe it means you could avoid
renaming the existing function instead (and add the "lck" postfix to
the new _wrapper_ function, since you need to change the line that
initializes things anyway)

 - in this case, I think the prototype would probably be improved by
passing in the Scsi_Host pointer. Both the caller and the callee _do_
need it, after all. So yes, it's an "artificial" change, but it's one
that doesn't make the code look worse.

But really, I don't care deeply. I think changing the name of the
"queuecommand" function in scsi_host_template (or whatever) is fine
too, and will certainly fix the "silently and happily compile even
non-converted code" problem too. That _is_ the more common way of
handling this, but as mentioned above, we've had issues with that
approach too. So whatever...

                           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/