Re: Bug and patch for BSD partition detection

[Heikki Vatiainen]

"Jochen Hoenicke" <address@hidden> wrote:

> I don't know much about BSD slices and don't have any to test them,
> but it looks that your patch breaks some other things, e.g. the
> geometry command (are BSD partition still printed?).

Actually, it was the geometry command that made me to investigate 
what had changed between 0.5.95 and 0.5.96 grub versions. In my 
original mail, it shows how grub 0.5.96 only prints 'a' when it 
should print all the partitions like 0.5.95 does. After the patch, 
the geometry command started working.

> IS_PC_SLICE_TYPE_BSD() is sometimes used, to test if this is a kind of
> extended partition that contains other partitions.

That's what I was thinking also. However, currently the filesystem
type on that partition is causing problems. By masking the upper
bits off, only the partition type, not partition type + filesystem
type is looked at.

When I was looking for the problem, I noticed that the argument
`type' for the macro IS_PC_SLICE_TYPE_BSD contains both BSD
filesystem type (FS_BSDFFS, FS_SWAP, etc) and the FDISK partition
type so that e.g. `type' for the FreeBSD swap partition, usually
letter b, is 0x1a5.

Since IS_PC_SLICE_TYPE_BSD calls IS_PC_SLICE_TYPE_BSD_WITH_FS with
second argument being 0, I came into conclusion that `type' should
be masked with 0xff in IS_PC_SLICE_TYPE_BSD because
IS_PC_SLICE_TYPE_BSD is not interested in filesystem type which is 
carried in bits > 8 in `type'.

This is what both macros in stage2/pc_slice.h look like now:

/* For convenience.  */
#define IS_PC_SLICE_TYPE_BSD_WITH_FS(type,fs)   \
  ((type) == (PC_SLICE_TYPE_FREEBSD | ((fs) << 8)) \
   || (type) == (PC_SLICE_TYPE_OPENBSD | ((fs) << 8)) \
   || (type) == (PC_SLICE_TYPE_NETBSD | (fs) << 8))

#define IS_PC_SLICE_TYPE_BSD(type)      IS_PC_SLICE_TYPE_BSD_WITH_FS(type,0)


Now, if we are currently looking at e.g. FreeBSD swap partition
'b', `type' is 0x1a5 and we will get incorrect result from
IS_PC_SLICE_TYPE_BSD. The reason 'a' is printed, is that once the
code notices that it is looking at a BSD partition, it starts
adding filesystem type into `type'.

> I think you should only mask out the type in the next_partition method
> as in the attached patch.  Does this fix the problem?

I think it will, but I have not been able to test it. The machine 
is currently at work and today is a national holiday. It is also 
running NetBSD and the CVS version of grub requires a version of 
binutils which is newer than what is installed. This is not fatal 
by itself, but the latest binutils does not work either:

checking whether /home/hessu/binutils/bin/objcopy works for absolute 
addresses...
     configure: error: gcc cannot link at address 2000
zsh: exit 1     ./configure --with-binutils=/home/hessu/binutils/bin
% ~/binutils/bin/as -v
GNU assembler version 2.10.1 (i386-unknown-netbsdelf1.5) using BFD version 
2.10.1

Heikki
-- 
Heikki Vatiainen                  * address@hidden
Tampere University of Technology  * Tampere, Finland