Re: [patch] memory commands

[Marco Gerards]

Hollis Blanchard <address@hidden> writes:

>> Personally I prefer to call this module `memdebug'.  In that case it
>> is clear that it is intended for debugging, not for other memory
>> related commands (which we might need someday, but are not related at
>> all).
>>
>>> Comments?
>>
>> I'd like to do a full review later in the weekend, if possible.
>
> Here is the patch with the module renamed to "memdebug".

Cool, thanks :)

> +2005-12-12  Hollis Blanchard  <address@hidden>
> +
> +     * commands/memdebug.c: New file.

> +     * kern/powerpc/misc.c: Likewise.
> +     * include/grub/powerpc/misc.h: Likewise.
> +     * include/grub/i386/misc.h: Likewise.
> +     * include/grub/sparc64/misc.h: Likewise.

Why don't you use trap.[ch]?  Calling it misc is a bit too generic I
think, and I'm afraid everyone put everything in it.


> +  grub_register_command ("memset", grub_cmd_memset, GRUB_COMMAND_FLAG_BOTH,
> +                      "memset address (value [len] | trap)",
> +                      "Write a 32-bit quantity to memory.", 0);

This syntax is a bit unusual, I think (for a GNU program).  Mainly
because it has quite some conditional parameters.  How about:

memset --value=len

or:

memset --trap

Can you please explain what this command does?  To be honest I don't
really understand it immediately from the sourcecode. ;-)

--
Marco