* [U-Boot] [PATCH v2] part_efi: fix protective mbr struct allocation
@ 2014-02-13 8:48 Hector Palacios
2014-02-13 13:06 ` Lukasz Majewski
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Hector Palacios @ 2014-02-13 8:48 UTC (permalink / raw)
To: u-boot
The calloc() call was allocating space for the sizeof the struct
pointer rather than for the struct contents.
Besides, since this buffer is passed to mmc for writing and some
platforms may use cache, the legacy_mbr struct should be cache-aligned.
Signed-off-by: Hector Palacios <hector.palacios@digi.com>
---
Notes:
Changes since V1:
- Cache-align declaration of p_mbr pointer
- Use *p_mbr in sizeof() to match kernel CodingStyle
disk/part_efi.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/disk/part_efi.c b/disk/part_efi.c
index 5dfaf490c89a..42936e04fb67 100644
--- a/disk/part_efi.c
+++ b/disk/part_efi.c
@@ -229,10 +229,10 @@ int test_part_efi(block_dev_desc_t * dev_desc)
*/
static int set_protective_mbr(block_dev_desc_t *dev_desc)
{
- legacy_mbr *p_mbr;
-
/* Setup the Protective MBR */
- p_mbr = calloc(1, sizeof(p_mbr));
+ ALLOC_CACHE_ALIGN_BUFFER(legacy_mbr, p_mbr, 1);
+ memset(p_mbr, 0, sizeof(*p_mbr));
+
if (p_mbr == NULL) {
printf("%s: calloc failed!\n", __func__);
return -1;
@@ -247,11 +247,9 @@ static int set_protective_mbr(block_dev_desc_t *dev_desc)
if (dev_desc->block_write(dev_desc->dev, 0, 1, p_mbr) != 1) {
printf("** Can't write to device %d **\n",
dev_desc->dev);
- free(p_mbr);
return -1;
}
- free(p_mbr);
return 0;
}
^ permalink raw reply related [flat|nested] 5+ messages in thread* [U-Boot] [PATCH v2] part_efi: fix protective mbr struct allocation
2014-02-13 8:48 [U-Boot] [PATCH v2] part_efi: fix protective mbr struct allocation Hector Palacios
@ 2014-02-13 13:06 ` Lukasz Majewski
2014-02-24 15:46 ` Lukasz Majewski
2014-02-25 13:57 ` [U-Boot] [U-Boot, " Tom Rini
2 siblings, 0 replies; 5+ messages in thread
From: Lukasz Majewski @ 2014-02-13 13:06 UTC (permalink / raw)
To: u-boot
Hi Hector,
> The calloc() call was allocating space for the sizeof the struct
> pointer rather than for the struct contents.
> Besides, since this buffer is passed to mmc for writing and some
> platforms may use cache, the legacy_mbr struct should be
> cache-aligned.
Thanks for preparing v2.
Everything seems OK.
Test HW: Exynos4210 - Trats.
Tested-by: Lukasz Majewski <l.majewski@samsung.com>
>
> Signed-off-by: Hector Palacios <hector.palacios@digi.com>
> ---
>
> Notes:
> Changes since V1:
> - Cache-align declaration of p_mbr pointer
> - Use *p_mbr in sizeof() to match kernel CodingStyle
>
> disk/part_efi.c | 8 +++-----
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/disk/part_efi.c b/disk/part_efi.c
> index 5dfaf490c89a..42936e04fb67 100644
> --- a/disk/part_efi.c
> +++ b/disk/part_efi.c
> @@ -229,10 +229,10 @@ int test_part_efi(block_dev_desc_t * dev_desc)
> */
> static int set_protective_mbr(block_dev_desc_t *dev_desc)
> {
> - legacy_mbr *p_mbr;
> -
> /* Setup the Protective MBR */
> - p_mbr = calloc(1, sizeof(p_mbr));
> + ALLOC_CACHE_ALIGN_BUFFER(legacy_mbr, p_mbr, 1);
> + memset(p_mbr, 0, sizeof(*p_mbr));
> +
> if (p_mbr == NULL) {
> printf("%s: calloc failed!\n", __func__);
> return -1;
> @@ -247,11 +247,9 @@ static int set_protective_mbr(block_dev_desc_t
> *dev_desc) if (dev_desc->block_write(dev_desc->dev, 0, 1, p_mbr) !=
> 1) { printf("** Can't write to device %d **\n",
> dev_desc->dev);
> - free(p_mbr);
> return -1;
> }
>
> - free(p_mbr);
> return 0;
> }
>
--
Best regards,
Lukasz Majewski
Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
^ permalink raw reply [flat|nested] 5+ messages in thread* [U-Boot] [PATCH v2] part_efi: fix protective mbr struct allocation
2014-02-13 8:48 [U-Boot] [PATCH v2] part_efi: fix protective mbr struct allocation Hector Palacios
2014-02-13 13:06 ` Lukasz Majewski
@ 2014-02-24 15:46 ` Lukasz Majewski
2014-02-24 15:52 ` Tom Rini
2014-02-25 13:57 ` [U-Boot] [U-Boot, " Tom Rini
2 siblings, 1 reply; 5+ messages in thread
From: Lukasz Majewski @ 2014-02-24 15:46 UTC (permalink / raw)
To: u-boot
Hi Tom,
> The calloc() call was allocating space for the sizeof the struct
> pointer rather than for the struct contents.
> Besides, since this buffer is passed to mmc for writing and some
> platforms may use cache, the legacy_mbr struct should be
> cache-aligned.
Is there any problem with this patch?
>
> Signed-off-by: Hector Palacios <hector.palacios@digi.com>
> ---
>
> Notes:
> Changes since V1:
> - Cache-align declaration of p_mbr pointer
> - Use *p_mbr in sizeof() to match kernel CodingStyle
>
> disk/part_efi.c | 8 +++-----
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/disk/part_efi.c b/disk/part_efi.c
> index 5dfaf490c89a..42936e04fb67 100644
> --- a/disk/part_efi.c
> +++ b/disk/part_efi.c
> @@ -229,10 +229,10 @@ int test_part_efi(block_dev_desc_t * dev_desc)
> */
> static int set_protective_mbr(block_dev_desc_t *dev_desc)
> {
> - legacy_mbr *p_mbr;
> -
> /* Setup the Protective MBR */
> - p_mbr = calloc(1, sizeof(p_mbr));
> + ALLOC_CACHE_ALIGN_BUFFER(legacy_mbr, p_mbr, 1);
> + memset(p_mbr, 0, sizeof(*p_mbr));
> +
> if (p_mbr == NULL) {
> printf("%s: calloc failed!\n", __func__);
> return -1;
> @@ -247,11 +247,9 @@ static int set_protective_mbr(block_dev_desc_t
> *dev_desc) if (dev_desc->block_write(dev_desc->dev, 0, 1, p_mbr) !=
> 1) { printf("** Can't write to device %d **\n",
> dev_desc->dev);
> - free(p_mbr);
> return -1;
> }
>
> - free(p_mbr);
> return 0;
> }
>
--
Best regards,
Lukasz Majewski
Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
^ permalink raw reply [flat|nested] 5+ messages in thread* [U-Boot] [PATCH v2] part_efi: fix protective mbr struct allocation
2014-02-24 15:46 ` Lukasz Majewski
@ 2014-02-24 15:52 ` Tom Rini
0 siblings, 0 replies; 5+ messages in thread
From: Tom Rini @ 2014-02-24 15:52 UTC (permalink / raw)
To: u-boot
On Mon, Feb 24, 2014 at 04:46:05PM +0100, Lukasz Majewski wrote:
> Hi Tom,
>
> > The calloc() call was allocating space for the sizeof the struct
> > pointer rather than for the struct contents.
> > Besides, since this buffer is passed to mmc for writing and some
> > platforms may use cache, the legacy_mbr struct should be
> > cache-aligned.
>
> Is there any problem with this patch?
Just started re-reviewing this one today in fact, good timing.
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140224/1644ca72/attachment.pgp>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [U-Boot] [U-Boot, v2] part_efi: fix protective mbr struct allocation
2014-02-13 8:48 [U-Boot] [PATCH v2] part_efi: fix protective mbr struct allocation Hector Palacios
2014-02-13 13:06 ` Lukasz Majewski
2014-02-24 15:46 ` Lukasz Majewski
@ 2014-02-25 13:57 ` Tom Rini
2 siblings, 0 replies; 5+ messages in thread
From: Tom Rini @ 2014-02-25 13:57 UTC (permalink / raw)
To: u-boot
On Thu, Feb 13, 2014 at 09:48:24AM +0100, Hector Palacios wrote:
> The calloc() call was allocating space for the sizeof the struct
> pointer rather than for the struct contents.
> Besides, since this buffer is passed to mmc for writing and some
> platforms may use cache, the legacy_mbr struct should be cache-aligned.
>
> Signed-off-by: Hector Palacios <hector.palacios@digi.com>
> Tested-by: Lukasz Majewski <l.majewski@samsung.com>
Applied to u-boot/master, thanks!
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140225/19435031/attachment.pgp>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-02-25 13:57 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-13 8:48 [U-Boot] [PATCH v2] part_efi: fix protective mbr struct allocation Hector Palacios
2014-02-13 13:06 ` Lukasz Majewski
2014-02-24 15:46 ` Lukasz Majewski
2014-02-24 15:52 ` Tom Rini
2014-02-25 13:57 ` [U-Boot] [U-Boot, " Tom Rini
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.