grub-devel
[Top][All Lists]
Advanced

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

Re: test -e patch


From: adrian15
Subject: Re: test -e patch
Date: Sun, 03 Jun 2007 04:59:44 +0200
User-agent: Mozilla Thunderbird 1.0.7 (Windows/20050923)

On Wed, Jun 06, 2007 at 01:42:48AM +0200, adrian15 wrote:
Attached you will find the patch adding test -e support for grub2.

This is my first patch. I have compiled it without no errors.

However as long as the grub2.tar.gz that Marco gave me did not have any
documentation about how to create a floppy (or at least I did not manage
to find it)

This should work:

  cat boot.img core.img | dd of=foo.img seek=0 conv=notrunc
I would like to have a floppy with the fs on it. See my other mail about
my floppy creation adventure.

   > static const struct grub_arg_option options[] =
 {
   {"file", 'e', 0, "test if a file exists", 0, 0},
   {0, 0, 0, 0, 0, 0}
 };
Is this correct? What's that last line for? Is it compulsory ?

It's common practice for arrays to make them null-terminated to be able to
determine their limits without making assumptions about their size.  Doing it
with structs is akin to null-terminating a char array.
Ok. I understand then.

{
        grub_file_t file;

     file = grub_file_open (key);

What happens if file is a device node, or a directory?  Does grub_file_open
work with these?
I have not tried a device node, but with a directory it fails.

 Perhaps grub_file_stat would make more sense
You might be right but why then you (grub2 developers) have developed
the search command with the help of grub_file_open ??

Should the search command be reimplemented with grub_file_stat ?

What are the error polices ?
IMHO, an error would be if existance of the file cannot be determined.  I don't
think non-existance should be considered an error too.  However, existing code
in the test plugin already does this, which puzzles me.
We will have to wait for marco_g for telling us how it works, then.

GRUB_MOD_INIT(test)
{
 (void)mod;                     /* To stop warning. */
 grub_register_command ("[", grub_cmd_test, GRUB_COMMAND_FLAG_CMDLINE,
                         "[ EXPRESSION ]", "Evaluate an expression", 0);
 grub_register_command ("test", grub_cmd_test, GRUB_COMMAND_FLAG_CMDLINE,
                         "test EXPRESSION", "Evaluate an expression", 0);
}
I understand this register commands. I suppose this information is read
from the help command or it isn't ?

Yes.  Also with "test --help".

Or maybe it also reads from:
static const struct grub_arg_option options ?

That too.

The question is if the user will see the -e, -f or other options when
querying the test command help or not ?

Yes, based on options[].  But you should really test that and not blindly
believe me :-)
I've checked that it does not read from struct grub_arg_option options ,
strange thing.

+static const struct grub_arg_option options[] =
+  {
+    {"file", 'e', 0, "Test if a file exists", 0, 0},

Do we want a long option here?  bash/coreutils don't have any.

I do not want a long option :)

adrian15





reply via email to

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