From: Daniel Kiper <dkiper@net-space.pl>
To: Vladimir 'phcoder' Serbinenko <phcoder@gmail.com>
Cc: The development of GRUB 2 <grub-devel@gnu.org>, ardb@kernel.org
Subject: Re: [PATCH 3/4] types: Split aligned and packed guids
Date: Tue, 17 Oct 2023 18:41:18 +0200 [thread overview]
Message-ID: <20231017164118.5lrwcox7b37pfq6x@tomti.i.net-space.pl> (raw)
In-Reply-To: <CAEaD8JMFQDzuP_WDO2qjN=WfGBWuNd0YNKC60woA-i2zAoro0Q@mail.gmail.com>
Adding Ard...
On Sat, Oct 07, 2023 at 05:06:46PM +0200, Vladimir 'phcoder' Serbinenko wrote:
> From 8f4a7c92ffe17dd674e80bd3beacb2dac03d135b Mon Sep 17 00:00:00 2001
> From: Vladimir Serbinenko <phcoder@gmail.com>
> Date: Sun, 13 Aug 2023 09:18:23 +0200
> Subject: [PATCH 3/4] types: Split aligned and packed guids
>
> Signed-off-by: Vladimir Serbinenko <phcoder@gmail.com>
> ---
> grub-core/commands/probe.c | 12 ++++++------
> grub-core/kern/misc.c | 24 ++++++++++++------------
> include/grub/efi/api.h | 2 +-
> include/grub/types.h | 8 +++++++-
> 4 files changed, 26 insertions(+), 20 deletions(-)
>
> diff --git a/grub-core/commands/probe.c b/grub-core/commands/probe.c
> index be9637f33..193957c39 100644
> --- a/grub-core/commands/probe.c
> +++ b/grub-core/commands/probe.c
> @@ -119,7 +119,7 @@ grub_cmd_probe (grub_extcmd_context_t ctxt, int argc, char **args)
> if (grub_strcmp(dev->disk->partition->partmap->name, "gpt") == 0)
> {
> struct grub_gpt_partentry entry;
> - grub_guid_t *guid;
> + grub_guid_t guid;
>
> if (grub_disk_read(disk, p->offset, p->index, sizeof(entry), &entry))
> {
> @@ -129,11 +129,11 @@ grub_cmd_probe (grub_extcmd_context_t ctxt, int argc, char **args)
> grub_error_pop ();
> return grub_errno;
> }
> - guid = &entry.guid;
> - guid->data1 = grub_le_to_cpu32 (guid->data1);
> - guid->data2 = grub_le_to_cpu16 (guid->data2);
> - guid->data3 = grub_le_to_cpu16 (guid->data3);
> - grub_snprintf (val, sizeof(val), "%pG", guid);
> + guid = entry.guid;
> + guid.data1 = grub_le_to_cpu32 (guid.data1);
> + guid.data2 = grub_le_to_cpu16 (guid.data2);
> + guid.data3 = grub_le_to_cpu16 (guid.data3);
> + grub_snprintf (val, sizeof(val), "%pG", &guid);
> }
> else if (grub_strcmp(dev->disk->partition->partmap->name, "msdos") == 0)
> {
> diff --git a/grub-core/kern/misc.c b/grub-core/kern/misc.c
> index b57249acb..d4daacf65 100644
> --- a/grub-core/kern/misc.c
> +++ b/grub-core/kern/misc.c
> @@ -1068,22 +1068,22 @@ grub_vsnprintf_real (char *str, grub_size_t max_len, const char *fmt0,
> if (*(fmt) == 'G')
> {
> ++fmt;
> - grub_guid_t *guid = (grub_guid_t *)(grub_addr_t) curarg;
> - write_number (str, &count, max_len, 8, 0, '0', 'x', guid->data1);
> + grub_packed_guid_t *guid = (grub_packed_guid_t *)(grub_addr_t) curarg;
> + write_number (str, &count, max_len, 8, 0, '0', 'x', guid->inner.data1);
> write_char (str, &count, max_len, '-');
> - write_number (str, &count, max_len, 4, 0, '0', 'x', guid->data2);
> + write_number (str, &count, max_len, 4, 0, '0', 'x', guid->inner.data2);
> write_char (str, &count, max_len, '-');
> - write_number (str, &count, max_len, 4, 0, '0', 'x', guid->data3);
> + write_number (str, &count, max_len, 4, 0, '0', 'x', guid->inner.data3);
> write_char (str, &count, max_len, '-');
> - write_number (str, &count, max_len, 2, 0, '0', 'x', guid->data4[0]);
> - write_number (str, &count, max_len, 2, 0, '0', 'x', guid->data4[1]);
> + write_number (str, &count, max_len, 2, 0, '0', 'x', guid->inner.data4[0]);
> + write_number (str, &count, max_len, 2, 0, '0', 'x', guid->inner.data4[1]);
> write_char (str, &count, max_len, '-');
> - write_number (str, &count, max_len, 2, 0, '0', 'x', guid->data4[2]);
> - write_number (str, &count, max_len, 2, 0, '0', 'x', guid->data4[3]);
> - write_number (str, &count, max_len, 2, 0, '0', 'x', guid->data4[4]);
> - write_number (str, &count, max_len, 2, 0, '0', 'x', guid->data4[5]);
> - write_number (str, &count, max_len, 2, 0, '0', 'x', guid->data4[6]);
> - write_number (str, &count, max_len, 2, 0, '0', 'x', guid->data4[7]);
> + write_number (str, &count, max_len, 2, 0, '0', 'x', guid->inner.data4[2]);
> + write_number (str, &count, max_len, 2, 0, '0', 'x', guid->inner.data4[3]);
> + write_number (str, &count, max_len, 2, 0, '0', 'x', guid->inner.data4[4]);
> + write_number (str, &count, max_len, 2, 0, '0', 'x', guid->inner.data4[5]);
> + write_number (str, &count, max_len, 2, 0, '0', 'x', guid->inner.data4[6]);
> + write_number (str, &count, max_len, 2, 0, '0', 'x', guid->inner.data4[7]);
> break;
> }
> else
> diff --git a/include/grub/efi/api.h b/include/grub/efi/api.h
> index d3eaef3fb..0ff956adc 100644
> --- a/include/grub/efi/api.h
> +++ b/include/grub/efi/api.h
> @@ -1388,7 +1388,7 @@ struct grub_efi_configuration_table
> {
> grub_guid_t vendor_guid;
> void *vendor_table;
> -} GRUB_PACKED;
> +};
If it is correct then it requires an explanation in the commit message.
And what about the other types which have grub_guid_t internally?
> typedef struct grub_efi_configuration_table grub_efi_configuration_table_t;
>
> #define GRUB_EFIEMU_SYSTEM_TABLE_SIGNATURE 0x5453595320494249LL
> diff --git a/include/grub/types.h b/include/grub/types.h
> index 0d96006fe..ce7fa898f 100644
> --- a/include/grub/types.h
> +++ b/include/grub/types.h
> @@ -376,7 +376,13 @@ struct grub_guid
> grub_uint16_t data2;
> grub_uint16_t data3;
> grub_uint8_t data4[8];
> -} GRUB_PACKED;
> +};
Why not __attribute__ ((__aligned__ (__alignof__ (grub_uint32_t))))? [1] [2]
> typedef struct grub_guid grub_guid_t;
>
> +struct grub_packed_guid
> +{
> + struct grub_guid inner;
> +} __attribute__ ((packed));
s/__attribute__ ((packed))/GRUB_PACKED/
Daniel
[1] https://github.com/torvalds/linux/blob/master/include/linux/efi.h#L60-L78
[2] https://github.com/torvalds/linux/commit/494c704f9af0a0cddf593b381ea44320888733e6
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
next prev parent reply other threads:[~2023-10-17 16:41 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-07 15:06 [PATCH 3/4] types: Split aligned and packed guids Vladimir 'phcoder' Serbinenko
2023-10-17 16:41 ` Daniel Kiper [this message]
-- strict thread matches above, loose matches on Subject: below --
2023-10-31 19:35 Vladimir 'phcoder' Serbinenko
2023-10-31 21:43 ` John Paul Adrian Glaubitz
2023-11-06 17:49 ` Daniel Kiper
2023-11-06 21:59 ` Vladimir 'phcoder' Serbinenko
2023-11-07 18:32 ` Daniel Kiper
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=20231017164118.5lrwcox7b37pfq6x@tomti.i.net-space.pl \
--to=dkiper@net-space.pl \
--cc=ardb@kernel.org \
--cc=grub-devel@gnu.org \
--cc=phcoder@gmail.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).