Re: Partition modules

[Hollis Blanchard]

On Nov 17, 2004, at 1:25 PM, Marco Gerards wrote:
> Here is a patch to modulize partition support.  At the moment it will
> create pc.mod, amiga.mod and apple.mod.  Loading one of these modules
> will give support for that specific partition map.

Thanks Marco!

> +/* Partition description.  */
> +struct grub_partition
> +{
> +  /* The start sector.  */
> +  unsigned long start;
> +
> +  /* The length in sector units.  */
> +  unsigned long len;
> +
> +  /* The offset of the partition table.  */
> +  unsigned long offset;
> +
> +  /* The index of this partition in the partition table.  */
> +  int index;
> +
> +  /* The DOS partition number.  */
> +  int dos_part;
> +
> +  /* The BSD partition number (a == 0).  */
> +  int bsd_part;
> +
> +  /* The DOS partition type.  */
> +  int dos_type;
> +
> +  /* The BSD partition type.  */
> +  int bsd_type;
> +
> +  /* The offset of the extended partition.  */
> +  unsigned long ext_offset;
> +
> +  /* The type partition map.  */
> +  grub_partition_map_t partmap;
> +};

I do have an objection here: members like "bsd_type" don't belong in a 
generic struct, but rather in grub_partition_bsd_type. I believe 
grub_partition should be the abstraction, i.e. what defines a partition 
in the general sense. start, len, and index are obviously good; offset 
is compensating for a DOS issue, though at least it has (irrelevant) 
meaning for an Apple map... but ext_offset and these dos_ and bsd_ 
things don't belong here IMHO.

Also, there were a number of whitespace-only lines in your patch; a 
simple regex should show you where.

I'm seeing a crash on both briQ and my G3, so I'm sprinkling printfs 
now...

-Hollis