Messages in this thread Patch in this message | | | Subject | Re: [PATCH 2.6.7-mm1] MBR centralization | From | FabF <> | Date | Thu, 24 Jun 2004 17:10:14 +0200 |
| |
On Thu, 2004-06-24 at 09:38, Anton Altaparmakov wrote: > On Thu, 24 Jun 2004, Anton Altaparmakov wrote: > > On Thu, 24 Jun 2004, FabF wrote: > >> Here's a new version: > >> -macro simplification (We're calling from struct & buffer so static > >> fx > >> conversion seems troublesome) > >> -(p) > >> -MBR patch only > > > > Your patch is wrong: > > > > diff -Naur orig/include/linux/genhd.h edited/include/linux/genhd.h > > --- orig/include/linux/genhd.h 2004-06-16 07:18:59.000000000 +0200 > > +++ edited/include/linux/genhd.h 2004-06-24 08:02:45.151489490 +0200 > > @@ -17,6 +17,9 @@ > > #include <linux/string.h> > > #include <linux/fs.h> > > > > +/*Master boot record magic numbers*/ > > +#define MSDOS_MBR(p) le16_to_cpu((u16)*(p)) == 0xaa55 > > > > This macro is completely broken for all p with a type size other than 16 > > bits. E.g. if you have "char *p" then *(p) gives you a single byte which you > > then cast to u16 so you will never see the 0xaa55. > > > > If you want to do it this way you need to cast p to (u16*) first like so: > > > > #define MSDOS_MBR(p) (le16_to_cpup((u16*)(p)) == 0xaa55) > > > > Note using _cpup() means you don't have to do the dereferencing yourself > > which looks much cleaner IMO. You do still need the cast as it is missing > > from various macros in the kernel (I consider this a bug but this is > > orthogonal to this patch). > > While I am at it, the above macro is even further optimized by moving the > endianness conversion to the constant so it is applied at compile time > rather than run time like so: > > #define MSDOS_MBR(p) ((*(u16*)(p)) == __constant_cpu_to_le16(0xaa55) Thanks Anton :) Applied.It worked before but in semi-evaluation.
Regards, FabF
> > Best regards, > > Anton diff -Naur orig/fs/partitions/efi.c edited/fs/partitions/efi.c --- orig/fs/partitions/efi.c 2004-06-16 07:19:23.000000000 +0200 +++ edited/fs/partitions/efi.c 2004-06-24 01:43:45.000000000 +0200 @@ -145,7 +145,7 @@ int i, found = 0, signature = 0; if (!mbr) return 0; - signature = (le16_to_cpu(mbr->signature) == MSDOS_MBR_SIGNATURE); + signature = MSDOS_MBR(&mbr->signature); for (i = 0; signature && i < 4; i++) { if (mbr->partition_record[i].sys_ind == EFI_PMBR_OSTYPE_EFI_GPT) { diff -Naur orig/fs/partitions/efi.h edited/fs/partitions/efi.h --- orig/fs/partitions/efi.h 2004-06-16 07:19:43.000000000 +0200 +++ edited/fs/partitions/efi.h 2004-06-24 01:20:46.000000000 +0200 @@ -34,7 +34,6 @@ #include <linux/string.h> #include <linux/efi.h> -#define MSDOS_MBR_SIGNATURE 0xaa55 #define EFI_PMBR_OSTYPE_EFI 0xEF #define EFI_PMBR_OSTYPE_EFI_GPT 0xEE diff -Naur orig/fs/partitions/msdos.c edited/fs/partitions/msdos.c --- orig/fs/partitions/msdos.c 2004-06-16 07:19:37.000000000 +0200 +++ edited/fs/partitions/msdos.c 2004-06-24 08:00:05.000000000 +0200 @@ -50,15 +50,6 @@ SYS_IND(p) == LINUX_EXTENDED_PARTITION); } -#define MSDOS_LABEL_MAGIC1 0x55 -#define MSDOS_LABEL_MAGIC2 0xAA - -static inline int -msdos_magic_present(unsigned char *p) -{ - return (p[0] == MSDOS_LABEL_MAGIC1 && p[1] == MSDOS_LABEL_MAGIC2); -} - /* * Create devices for each logical partition in an extended partition. * The logical partitions form a linked list, with each entry being @@ -87,6 +78,7 @@ this_size = first_size; while (1) { + /*FIXME : Some definition for 100*/ if (++loopct > 100) return; if (state->next == state->limit) @@ -95,7 +87,7 @@ if (!data) return; - if (!msdos_magic_present(data + 510)) + if (!MSDOS_MBR (data+510)) goto done; p = (struct partition *) (data + 0x1be); @@ -112,7 +104,7 @@ /* * First process the data partition(s) */ - for (i=0; i<4; i++, p++) { + for (i = 0; i < 4; i++, p++) { u32 offs, size, next; if (!NR_SECTS(p) || is_extended_partition(p)) continue; @@ -146,7 +138,7 @@ * It should be a link to the next logical partition. */ p -= 4; - for (i=0; i<4; i++, p++) + for (i = 0; i < 4; i++, p++) if (NR_SECTS(p) && is_extended_partition(p)) break; if (i == 4) @@ -342,7 +334,7 @@ /* The first sector of a Minix partition can have either * a secondary MBR describing its subpartitions, or * the normal boot sector. */ - if (msdos_magic_present (data + 510) && + if (MSDOS_MBR(data + 510) && SYS_IND(p) == MINIX_PARTITION) { /* subpartition table present */ printk(" %s%d: <minix:", state->name, origin); @@ -385,7 +377,7 @@ data = read_dev_sector(bdev, 0, §); if (!data) return -1; - if (!msdos_magic_present(data + 510)) { + if (!MSDOS_MBR(data + 510)) { put_dev_sector(sect); return 0; } @@ -403,7 +395,7 @@ return 0; } } - + /*EFI : Extensible Firmware Initiative partitions ?*/ #ifdef CONFIG_EFI_PARTITION p = (struct partition *) (data + 0x1be); for (slot = 1 ; slot <= 4 ; slot++, p++) { @@ -433,6 +425,7 @@ extended partition, but leave room for LILO */ put_partition(state, slot, start, size == 1 ? 1 : 2); printk(" <"); + /*parsing extended for logical partitions*/ parse_extended(state, bdev, start, size); printk(" >"); continue; diff -Naur orig/include/linux/genhd.h edited/include/linux/genhd.h --- orig/include/linux/genhd.h 2004-06-16 07:18:59.000000000 +0200 +++ edited/include/linux/genhd.h 2004-06-24 17:11:58.447759035 +0200 @@ -17,6 +17,9 @@ #include <linux/string.h> #include <linux/fs.h> +/*Master boot record magic numbers*/ +#define MSDOS_MBR(p) ((*(u16*)(p)) == __constant_cpu_to_le16(0xaa55)) + enum { /* These three have identical behaviour; use the second one if DOS FDISK gets confused about extended/logical partitions starting past cylinder 1023. */ | |