On Wednesday December 6, kressb@fsc-usa.com wrote:
> Peter Samuelson wrote:
> >
> > [Roberto Ragusa]
> > > BTW, here is a little patch regarding a silly problem I found
> > > about RAID partitions naming (/proc/partitions).
> > > No more "md8" "md9" "md:" "md;" ... but "md8" "md9" "md10" "md11" ...
> > > Well, this patch should work up to "md99".
> >
> > This stuff *really* should be split out into the drivers. Brian Kress
> > had a patch against test11 for this. Brian? You want to fold in this
> > fix?
>
> Sure. I got resounding silence to posting the patch last time,
> so I'm not sure if anyone actually wants this patch, but here it is
> again with this fix (the non ugly version) folded in.
>
Have you ever noticed that bad patches get a lot more response than
good patches? I think people like having something useful to say and
a simple "I like it" often doesn't seem worth it, though in reality is
very valuable.
Mayge the thing to do is include a few obvious but not very
significant errors like:
- initialise a static variable to 0
- use /** to introduce a comment that isn't in the right format for
the auto-documentation stuff
- uses lines wider than 80 characters
- use unnecessary extra parentheses
- use non-standard indenting
that way people will respond because they think they have something to
say, and will hopefully comment further... :-)
But I see that you have done that .... a simple compiler error (I
think).
> diff -u --recursive linux-2.4.0-test11/fs/partitions/check.c
> linux-2.4.0-test11-ppfix/fs/partitions/check.c
> --- linux-2.4.0-test11/fs/partitions/check.c Mon Nov 20 15:17:27 2000
> +++ linux-2.4.0-test11-ppfix/fs/partitions/check.c Thu Nov 23 14:30:45
> 2000
> @@ -83,11 +83,10 @@
> */
> char *disk_name (struct gendisk *hd, int minor, char *buf)
> {
> - unsigned int part;
> const char *maj = hd->major_name;
> int unit = (minor >> hd->minor_shift) + 'a';
> + unsigned int part = minor & ((1 << hd->minor_shift) - 1);
>
> - part = minor & ((1 << hd->minor_shift) - 1);
here we have lost the "part" automatic variable in disk_name but ....
(stuff deleted)
> - else
> - sprintf(buf, "%s/c%dd%dp%d", maj, ctlr, disk, part);
> - return buf;
> - }
> + if (hd->hd_name) return hd->hd_name(hd, minor, buf);
> +
> if (part)
> sprintf(buf, "%s%c%d", maj, unit, part);
here we use it. Does this compile?
But apart from this, I like it, and I think that it would be good if
it went into 2.4.
Maybe do one more iteration (or convince me that the above isn't a
bug), and ask for comment. I promise to test and respond.
Then send it to Linus, and explain what it does, and mention that it
fixes a real issue in lvm (I assume it does as someone said so. I
haven't actually looked into that) and leave it to him.
NeilBrown
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/
This archive was generated by hypermail 2b29 : Thu Dec 07 2000 - 21:00:16 EST