[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: normal mode chainloader
From: |
Marco Gerards |
Subject: |
Re: normal mode chainloader |
Date: |
Tue, 22 Jun 2004 22:25:43 +0200 |
User-agent: |
Gnus/5.1006 (Gnus v5.10.6) Emacs/21.3 (gnu/linux) |
address@hidden (Tomas Ebenlendr) writes:
> Hm, I forgot to append the new version. (It fixes only the GCS-compliancy,
> as you commented the patch).
Ok, here are the comments I still have, to be complete. These
comments are only related to the GCS.
In my other email I told you what I thought of the boot.mod
dependency. If that is worked out all you need is Okuji's blessing,
after that I will commit the patch (that reminds me about some other
patches I will commit real soon... :-/).
No need to go through this patch again. Please do what you can do
while fixing this patch to the outcome of the discussing about the
boot.mod dependency, I will do the rest before committing.
> * commands/boot.c (GRUB_MOD_INIT): Add symbol
> grub_boot_dependency, used forsemantic dependency of
> commands.
> * conf/i386-pc.rmk: Add i386/pc/chainloader_normal.c as
> chain.mod.
Please include (chain_mod_SOURCES) here, like you did with
(GRUB_MODE_INIT). Same for adding chain.mod to pkgdata_MODULES. You
should notice you renamed _chain.mod to chain.mod.
> * include/grub/i386/pc/loader.h (grub_rescue_cmd_chainloader):
> Delete prototype of rescue command chainloader.
Ok, although I prefer `Deleted prototype' (or short `Deleted').
> + grub_boot_dependency = 1; /* To let be loader commands dependent on this
> command
> + (don't let gcc optimize off this symbol) */
Please fix the comment. I would use:
/* To let be loader commands dependent on this command (don't let gcc
optimize off this symbol). */
grub_boot_dependency = 1;
(Notice the `.' and the two spaces)
> diff -rupN -x CVS grub2_x/include/grub/boot_cmd.h
> grub2_work/include/grub/boot_cmd.h
> --- grub2_x/include/grub/boot_cmd.h 1970-01-01 01:00:00.000000000 +0100
> +++ grub2_work/include/grub/boot_cmd.h 2004-06-21 01:33:53.000000000
> +0200
> @@ -0,0 +1,26 @@
> +/* This file is intended as command dependency */
> +/*
> + * GRUB -- GRand Unified Bootloader
> + * Copyright (C) 2002 Free Software Foundation, Inc.
copyright crap: You have to update the copyright years. In this case
add 2004. AFAIK you should not use short notations or ranges, just
list the years like:
* Copyright (C) 2002, 2004 Free Software Foundation, Inc.
> +typedef enum
> + {
> + GRUB_CHAINLOADER_FORCE = 0x1
> + }
> +grub_chainloader_flags_t;
I think this should be:
} grub_chainloader_flags_t;
> +void EXPORT_FUNC(grub_chainloader_cmd) (const char *
> file,grub_chainloader_flags_t flags);
A long line...
> +/* FIXME these commands should be divided to argument parsing and real work,
> and moved to
> + * respective headers*/
Please fix this comment.
> - if (signature != grub_le_to_cpu16 (0xaa55) && ! force)
> + if (signature != grub_le_to_cpu16 (0xaa55) && ! (flags &
> GRUB_CHAINLOADER_FORCE) )
A long lines. And the last parenthesis should not be separated with a
space. This would be better:
if (signature != grub_le_to_cpu16 (0xaa55)
&& ! (flags & GRUB_CHAINLOADER_FORCE))
> + grub_boot_dependency = 1; /* To be dependent on boot command
> + (don't let gcc optimize off this symbol) */
The formatting of the comments is not correct.
Thanks,
Marco
Re: normal mode chainloader, Tomas Ebenlendr, 2004/06/22
Copyright crap Was: Re: normal mode chainloader, Tomas Ebenlendr, 2004/06/23
Re: Copyright crap Was: Re: normal mode chainloader, Jeroen Dekkers, 2004/06/23
Re: Copyright crap Was: Re: normal mode chainloader, Tomas Ebenlendr, 2004/06/23
Re: normal mode chainloader, Tomas Ebenlendr, 2004/06/23
Re: normal mode chainloader, Marco Gerards, 2004/06/23
Re: normal mode chainloader, Tomas Ebenlendr, 2004/06/24
Re: normal mode chainloader, Marco Gerards, 2004/06/24