[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: GRUB2: *BSD and more patch

From: Marco Gerards
Subject: Re: GRUB2: *BSD and more patch
Date: Sat, 20 Mar 2004 00:55:36 +0100
User-agent: Gnus/5.1006 (Gnus v5.10.6) Emacs/21.3 (gnu/linux)

Sergey Matveychuk <address@hidden> writes:

> Here is a third version.


After reading your patch I found some other problems, sorry for not
looking good enough the first time.  Most problems I found are minor

If it is ok for Okuji I can make the changes I proposed (and have a
quick look at the changelog entry) and commit the patch.

> -- 
> Sem.
> diff -ruNp grub2/ChangeLog grub2.test/ChangeLog
> --- grub2/ChangeLog   Fri Mar 19 23:55:21 2004
> +++ grub2.test/ChangeLog      Sat Mar 20 01:38:47 2004
> @@ -1,3 +1,19 @@
> +2004-03-15  Sergey Matveychuk  <address@hidden>
> +
> +     * util/i386/pc/biosdisk.c: Added headers for BSD.

Please say exactly what header files you are including.

> +     (pupa_util_biosdisk_get_pupa_dev): Do not call

Please end a sentence with a ".".

> +     make_device_name(drive,-1-1) if not a block device.
> +     * util/i386/pc/getroot.c: Update copyright.

No need to notice the copyright update when changing years.  It is
only required when changing the copyright holders (Correct me if I am

> +     (find_root_device): Do not check for a block device.
> +     * util/misc.c: Include malloc.h only if it's needed.
> +     (pupa_memalign): Change memalign() with malloc() where unavailable.
> +     * util/pupa-emu.c: Include malloc.h only if it's needed.
> +     (main): Move pupa_util_biosdisk_init() above pupa_guess_root_device().

I do not see the changes in the changelog entry.  Same
for including config.h in misc.c.

This is only what I noticed at first.  Please make sure the changelog
entries are complete.

> -/* Define it to \"addr32\" or \"addr32;\" to make GAS happy */
> +/* Define it to "addr32" or "addr32;" to make GAS happy */

Why are you doing this?  I assume this is what the autotools

> -/* Define it to \"data32\" or \"data32;\" to make GAS happy */
> +/* Define it to "data32" or "data32;" to make GAS happy */

Same here.

> +AC_CHECK_HEADER(malloc.h)
> +AC_CHECK_FUNC(memalign)

This is what I meant, great!

> +#if !defined(__FreeBSD__)
>    if (! S_ISBLK (st.st_mode))
>      return make_device_name (drive, -1, -1);
> +#endif

make_device_name does not have to be called at all?

> -#elif defined(__GNU__)
> -  /* GNU uses "/dev/[hs]d[0-9]+(s[0-9]+[a-z]?)?".  */
> +#elif defined(__GNU__) || defined(__FreeBSD__) || defined(__NetBSD__) || 
> defined(__OpenBSD__)
> +  /* GNU uses "/dev/[ahsw]d[0-9]+(s[0-9]+[a-z]?)?".  */

Is this comment correct?  It says something about GNU, now you changed
the regexp. so it matches *BSD as well.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]