2013-10-04 16:02:33

by Roy Franz

[permalink] [raw]
Subject: [PATCH] Remove redundant and incorrect memset()

Remove a redundant memset() call from efi_relocate_kernel() that
was clearing memory that would be used by BSS in non-compressed
images loaded with this function. This clear was redundant with
the clearing done in the image itself, and also implemented incorrectly
with a 0 length.

Signed-off-by: Roy Franz <[email protected]>
Acked-by: Mark Salter <[email protected]>
---
drivers/firmware/efi/efi-stub-helper.c | 2 --
1 file changed, 2 deletions(-)

diff --git a/drivers/firmware/efi/efi-stub-helper.c b/drivers/firmware/efi/efi-stub-helper.c
index cc0581d..b6bffbf 100644
--- a/drivers/firmware/efi/efi-stub-helper.c
+++ b/drivers/firmware/efi/efi-stub-helper.c
@@ -567,8 +567,6 @@ static efi_status_t efi_relocate_kernel(efi_system_table_t *sys_table_arg,
* have been allocated by UEFI, so we can safely use memcpy.
*/
memcpy((void *)new_addr, (void *)cur_image_addr, image_size);
- /* Zero any extra space we may have allocated for BSS. */
- memset((void *)(new_addr + image_size), alloc_size - image_size, 0);

/* Return the new address of the relocated image. */
*image_addr = new_addr;
--
1.7.10.4


2013-10-04 17:33:17

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH] Remove redundant and incorrect memset()


* Roy Franz <[email protected]> wrote:

> Remove a redundant memset() call from efi_relocate_kernel() that
> was clearing memory that would be used by BSS in non-compressed
> images loaded with this function. This clear was redundant with
> the clearing done in the image itself, and also implemented incorrectly
> with a 0 length.
>
> Signed-off-by: Roy Franz <[email protected]>
> Acked-by: Mark Salter <[email protected]>

Note the title:

[PATCH] Remove redundant and incorrect memset()

if it's both redundant *and* incorrect then the other memset() must be
incorrect too and must be removed as well ;-)

I suspect you wanted the title to say:

[PATCH] Remove redundant memset()

?

Thanks,

Ingo

2013-10-04 17:38:45

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH] Remove redundant and incorrect memset()

On 10/04/2013 10:33 AM, Ingo Molnar wrote:
>
> * Roy Franz <[email protected]> wrote:
>
>> Remove a redundant memset() call from efi_relocate_kernel() that
>> was clearing memory that would be used by BSS in non-compressed
>> images loaded with this function. This clear was redundant with
>> the clearing done in the image itself, and also implemented incorrectly
>> with a 0 length.
>>
>> Signed-off-by: Roy Franz <[email protected]>
>> Acked-by: Mark Salter <[email protected]>
>
> Note the title:
>
> [PATCH] Remove redundant and incorrect memset()
>
> if it's both redundant *and* incorrect then the other memset() must be
> incorrect too and must be removed as well ;-)
>
> I suspect you wanted the title to say:
>
> [PATCH] Remove redundant memset()
>

And more importantly:

[PATCH] boot,efi: Remove redundant memset()

2013-10-04 17:39:28

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH] Remove redundant and incorrect memset()

* Roy Franz <[email protected]> wrote:

> On Oct 4, 2013 1:33 PM, "Ingo Molnar" <[email protected]> wrote:
> >
> >
> > * Roy Franz <[email protected]> wrote:
> >
> > > Remove a redundant memset() call from efi_relocate_kernel() that
> > > was clearing memory that would be used by BSS in non-compressed
> > > images loaded with this function. This clear was redundant with
> > > the clearing done in the image itself, and also implemented incorrectly
> > > with a 0 length.
> > >
> > > Signed-off-by: Roy Franz <[email protected]>
> > > Acked-by: Mark Salter <[email protected]>
> >
> > Note the title:
> >
> > [PATCH] Remove redundant and incorrect memset()
> >
> > if it's both redundant *and* incorrect then the other memset() must be
> > incorrect too and must be removed as well ;-)
> >
> > I suspect you wanted the title to say:
> >
> > [PATCH] Remove redundant memset()
> >
> > ?
> >
> > Thanks,
> >
> > Ingo
> Hi Ingo,
>
> The memset being removed is redundant with a correctly implemented
> memset in the kernel itself. The memset being removed is also incorrect,
> with a hard coded length of 0.

So it was redundantly clearing nothing? ;-)

Anyway, I agree, the title makes sense.

Thanks,

Ingo