linux-kernel - Re: [PATCH block#for-2.6.31 2/3] block: set rq->resid_len to blk_rq

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: <20090515112901.84d4dd97.zaitcev@redhat.com>
Date:	Fri, 15 May 2009 11:29:01 -0600
From:	Pete Zaitcev <zaitcev@...hat.com>
To:	Tejun Heo <htejun@...il.com>
Cc:	Jens Axboe <jens.axboe@...cle.com>,
	James Bottomley <James.Bottomley@...senPartnership.com>,
	Boaz Harrosh <bharrosh@...asas.com>,
	Linux Kernel <linux-kernel@...r.kernel.org>,
	linux-scsi <linux-scsi@...r.kernel.org>,
	IDE/ATA development list <linux-ide@...r.kernel.org>,
	Bartlomiej Zolnierkiewicz <bzolnier@...il.com>,
	Borislav Petkov <petkovbb@...glemail.com>,
	Sergei Shtylyov <sshtylyov@...mvista.com>,
	Eric Moore <Eric.Moore@....com>,
	"Darrick J. Wong" <djwong@...ibm.com>, zaitcev@...hat.com
Subject: Re: [PATCH block#for-2.6.31 2/3] block: set rq->resid_len to
 blk_rq_bytes() on issue

On Sat, 16 May 2009 00:18:42 +0900, Tejun Heo <htejun@...il.com> wrote:

> In commit c3a4d78c580de4edc9ef0f7c59812fb02ceb037f, while introducing
> rq->resid_len, the default value of residue count was changed from
> full count to zero. []

So it's not a residue anymore, right? You should've renamed it to
rq->count or something, then. Now we have this:

> +++ block/drivers/block/ub.c
> @@ -781,8 +781,7 @@ static void ub_rw_cmd_done(struct ub_dev
>  
>  	if (cmd->error == 0) {
>  		if (blk_pc_request(rq)) {
> -			if (cmd->act_len < blk_rq_bytes(rq))
> -				rq->resid_len = blk_rq_bytes(rq) - cmd->act_len;
> +			rq->resid_len -= min(cmd->act_len, rq->resid_len);
>  			scsi_status = 0;

You are subtracting resid_len from itself. Just how in the world
can this be correct?

Even it if is, in fact, correct, it's such an eggregious violation
of good style, that your good programmer's card is going to lose
a big coupon and have a hole punched in it.

This is not in Linus' tree yet, but I'm going to take a hard look
at this once it shows up.

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