Re: test -e patch

[adrian15 <adrian15@raulete.net>]

> 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