From: Pavel Roskin <proski@gnu.org>
To: The development of GRUB 2 <grub-devel@gnu.org>
Subject: Re: [PATCH] hfs+ uuid
Date: Mon, 08 Jun 2009 18:01:53 -0400 [thread overview]
Message-ID: <1244498513.25158.17.camel@mj> (raw)
In-Reply-To: <d7ead6de0906081350t6be2099lfe91e9910dcb4016@mail.gmail.com>
On Mon, 2009-06-08 at 22:50 +0200, Vladimir 'phcoder' Serbinenko wrote:
> Here is the improved patch. I deliberately ignored md5 comments
> because this part will be gone anyway whel Michael Gorven signs his
> copyright assignment and we incorporate luks patches
Please consider if it would be better to supply the filesystem UUID on
the command line rather than the device name. That would make xnu_uuid
leaner and more flexible. I think we need the "uuid" command that would
get the uuid and the "xnu_uuid" command that would convert it.
It would be great if you run xnu_uuid.c through indent. The coding
style is quite different from that used elsewhere in GRUB. Or at least
please strip trailing spaces.
"file name required" is a wrong error message. It should be "device
file required". I think it would be better to expand "fs" and "FS" in
error messages as "filesystem".
If the variable name is not specified, I think xnu_uuid should just
output the UUID without any explanations. Explanations belong to the
help, not to the normal output.
"(void)mod;" is not needed. The "buf" variable in grub_cmd_xnu_uuid()
is unused.
--
Regards,
Pavel Roskin
next prev parent reply other threads:[~2009-06-08 22:02 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-05-03 10:42 [PATCH] hfs+ uuid Vladimir 'phcoder' Serbinenko
2009-05-03 16:55 ` Pavel Roskin
2009-05-04 13:08 ` Vladimir 'phcoder' Serbinenko
2009-05-04 13:22 ` Pavel Roskin
2009-05-04 15:52 ` Vladimir 'phcoder' Serbinenko
2009-06-08 20:50 ` Vladimir 'phcoder' Serbinenko
2009-06-08 20:50 ` Vladimir 'phcoder' Serbinenko
2009-06-08 22:01 ` Pavel Roskin [this message]
2009-06-10 8:17 ` Vladimir 'phcoder' Serbinenko
2009-06-12 1:11 ` Pavel Roskin
2009-06-15 23:26 ` Vladimir 'phcoder' Serbinenko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1244498513.25158.17.camel@mj \
--to=proski@gnu.org \
--cc=grub-devel@gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.