mtd/drivers/mtd rfd_ftl.c,1.1,1.2
Jörn Engel
joern at wohnheim.fh-wedel.de
Fri Jun 24 12:52:43 EDT 2005
On Thu, 23 June 2005 15:50:15 +0100, sean at infradead.org wrote:
> +
> + for (i=0; i<SECTOR_SIZE; i++) {
> + if (buf[i])
> + break;
> + }
> +
> + if (i < SECTOR_SIZE) {
> + rc = do_writesect(dev, sector, buf, &old_addr);
> + if (rc)
> + goto err;
> + }
> + else
> + part->sector_map[sector] = -1;
I don't like this part too much. Effectively, you're using "i" as a
state variable. In this specific case, it's not too bad, but my
experience with state variable is that they always hurt.
Maybe something like this:
+ for (i=0; i<SECTOR_SIZE; i++) {
+ if (!buf[i])
+ continue;
+ rc = do_writesect(dev, sector, buf, &old_addr);
+ if (rc)
+ goto err;
+ }
+
+ if (i == SECTOR_SIZE)
+ part->sector_map[sector] = -1;
Or even:
+ for (i=0; i<SECTOR_SIZE; i++) {
+ if (!buf[i])
+ continue;
+ rc = do_writesect(dev, sector, buf, &old_addr);
+ if (rc)
+ goto err;
+ goto hit;
+ }
+
+ /* no hit */
+ part->sector_map[sector] = -1;
+hit:
Now we have one additional goto and your cs professor would fail you
in class, but code flow and data is nicely seperated.
Jörn
--
Mac is for working,
Linux is for Networking,
Windows is for Solitaire!
-- stolen from dc
More information about the linux-mtd
mailing list