Re: [PATCH] BFS support

[Robert Millan]

On Sat, Jul 18, 2009 at 12:59:45AM +0200, Vladimir 'phcoder' Serbinenko wrote:
> Hello. Here is a patch to add BFS support to GRUB2. BFS is a
> filesystem which was originally developped by Be, Inc for their
> proprietary BeOS. When Be Inc was sold to Palm and developpement of
> BeOS ceased a community formed to rewrite it from scratch under MIT
> license. Currently this project named Haiku is usable and interesting
> even if it for the moment lacks important feature to achieve its goal
> of use-friendly desktop OS. Currently they have plans to add multiboot
> support (see another thread). As AtheFS is a descendent of BeOS based
> on BFS driver from Haiku I used already existing afs driver and added
> necessary ifdefs. As you can see AFS and BFS are very similar

Looks very cleanly integrated.  I have a pair of comments:

> +#ifdef BFS
> +#define B_KEY_INDEX_ALIGN 8
> +#else
> +#define B_KEY_INDEX_ALIGN 4
> +#endif
> +
>  #define B_KEY_INDEX_OFFSET(node) ((grub_uint16_t *) \
> -                                   ((char *) (node) + \
> -                                    sizeof (struct grub_afs_bnode) + \
> -                                    ((node->key_size + 3) & ~3)))
> +                               ((char *) (node) \
> +                                + ((sizeof (struct grub_afs_bnode)   \
> +                                    + node->key_size + B_KEY_INDEX_ALIGN - 
> 1) \
> +                                   & ~(B_KEY_INDEX_ALIGN - 1))))

This looks like it would be a good case for using ALIGN_UP().

> +/* befs.c - The native BeOS/Haiku file-system.  */
> +#define BFS 1
> +#include "afs.c"

Can we name this macro consistently with the tar/cpio one?

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."