* [PATCH] kexec-tools: Don't duplicate the bzImage header area
@ 2011-02-08 15:20 Ahmed S. Darwish
2011-02-08 22:15 ` Simon Horman
0 siblings, 1 reply; 4+ messages in thread
From: Ahmed S. Darwish @ 2011-02-08 15:20 UTC (permalink / raw)
To: Simon Horman, Vivek Goyal, Haren Myneni, Eric Biederman; +Cc: kexec
Hi,
Don't wholeheartedly copy the 32-Kbytes bzImage header area to the stack,
just use a constant pointer instead.
Signed-off-by: Ahmed S. Darwish <darwish.07@gmail.com>
---
(If this is accepted, I'll send a similar patch for the setup header)
diff --git a/kexec/arch/i386/kexec-bzImage.c b/kexec/arch/i386/kexec-bzImage.c
index 83d3a69..2312eb8 100644
--- a/kexec/arch/i386/kexec-bzImage.c
+++ b/kexec/arch/i386/kexec-bzImage.c
@@ -43,32 +43,32 @@ static const int probe_debug = 0;
int bzImage_probe(const char *buf, off_t len)
{
- struct x86_linux_header header;
+ const struct x86_linux_header *header;
if ((uintmax_t)len < (uintmax_t)sizeof(header)) {
return -1;
}
- memcpy(&header, buf, sizeof(header));
- if (memcmp(header.header_magic, "HdrS", 4) != 0) {
+ header = (void *)buf;
+ if (memcmp(header->header_magic, "HdrS", 4) != 0) {
if (probe_debug) {
fprintf(stderr, "Not a bzImage\n");
}
return -1;
}
- if (header.boot_sector_magic != 0xAA55) {
+ if (header->boot_sector_magic != 0xAA55) {
if (probe_debug) {
fprintf(stderr, "No x86 boot sector present\n");
}
/* No x86 boot sector present */
return -1;
}
- if (header.protocol_version < 0x0200) {
+ if (header->protocol_version < 0x0200) {
if (probe_debug) {
fprintf(stderr, "Must be at least protocol version 2.00\n");
}
/* Must be at least protocol version 2.00 */
return -1;
}
- if ((header.loadflags & 1) == 0) {
+ if ((header->loadflags & 1) == 0) {
if (probe_debug) {
fprintf(stderr, "zImage not a bzImage\n");
}
thanks,
--
Darwish
http://darwish.07.googlepages.com
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] kexec-tools: Don't duplicate the bzImage header area
2011-02-08 15:20 [PATCH] kexec-tools: Don't duplicate the bzImage header area Ahmed S. Darwish
@ 2011-02-08 22:15 ` Simon Horman
2011-02-09 11:13 ` [PATCH v2] " Ahmed S. Darwish
0 siblings, 1 reply; 4+ messages in thread
From: Simon Horman @ 2011-02-08 22:15 UTC (permalink / raw)
To: Ahmed S. Darwish; +Cc: Haren Myneni, kexec, Eric Biederman, Vivek Goyal
On Tue, Feb 08, 2011 at 05:20:36PM +0200, Ahmed S. Darwish wrote:
> Hi,
>
> Don't wholeheartedly copy the 32-Kbytes bzImage header area to the stack,
> just use a constant pointer instead.
>
> Signed-off-by: Ahmed S. Darwish <darwish.07@gmail.com>
Hi Ahmed,
this seems reasonable to me, I have a minor comment below.
> ---
>
> (If this is accepted, I'll send a similar patch for the setup header)
>
> diff --git a/kexec/arch/i386/kexec-bzImage.c b/kexec/arch/i386/kexec-bzImage.c
> index 83d3a69..2312eb8 100644
> --- a/kexec/arch/i386/kexec-bzImage.c
> +++ b/kexec/arch/i386/kexec-bzImage.c
> @@ -43,32 +43,32 @@ static const int probe_debug = 0;
>
> int bzImage_probe(const char *buf, off_t len)
> {
> - struct x86_linux_header header;
> + const struct x86_linux_header *header;
> if ((uintmax_t)len < (uintmax_t)sizeof(header)) {
> return -1;
> }
> - memcpy(&header, buf, sizeof(header));
> - if (memcmp(header.header_magic, "HdrS", 4) != 0) {
> + header = (void *)buf;
Perhaps casting to (struct x86_linux_header*) would read better?
> + if (memcmp(header->header_magic, "HdrS", 4) != 0) {
> if (probe_debug) {
> fprintf(stderr, "Not a bzImage\n");
> }
> return -1;
> }
> - if (header.boot_sector_magic != 0xAA55) {
> + if (header->boot_sector_magic != 0xAA55) {
> if (probe_debug) {
> fprintf(stderr, "No x86 boot sector present\n");
> }
> /* No x86 boot sector present */
> return -1;
> }
> - if (header.protocol_version < 0x0200) {
> + if (header->protocol_version < 0x0200) {
> if (probe_debug) {
> fprintf(stderr, "Must be at least protocol version 2.00\n");
> }
> /* Must be at least protocol version 2.00 */
> return -1;
> }
> - if ((header.loadflags & 1) == 0) {
> + if ((header->loadflags & 1) == 0) {
> if (probe_debug) {
> fprintf(stderr, "zImage not a bzImage\n");
> }
>
> thanks,
>
> --
> Darwish
> http://darwish.07.googlepages.com
>
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v2] kexec-tools: Don't duplicate the bzImage header area
2011-02-08 22:15 ` Simon Horman
@ 2011-02-09 11:13 ` Ahmed S. Darwish
2011-02-09 23:25 ` Simon Horman
0 siblings, 1 reply; 4+ messages in thread
From: Ahmed S. Darwish @ 2011-02-09 11:13 UTC (permalink / raw)
To: Simon Horman; +Cc: Haren Myneni, kexec, Eric Biederman, Vivek Goyal
On Wed, Feb 09, 2011 at 07:15:45AM +0900, Simon Horman wrote:
> On Tue, Feb 08, 2011 at 05:20:36PM +0200, Ahmed S. Darwish wrote:
> >
> > diff --git a/kexec/arch/i386/kexec-bzImage.c b/kexec/arch/i386/kexec-bzImage.c
> > index 83d3a69..2312eb8 100644
> > --- a/kexec/arch/i386/kexec-bzImage.c
> > +++ b/kexec/arch/i386/kexec-bzImage.c
> > @@ -43,32 +43,32 @@ static const int probe_debug = 0;
> >
> > int bzImage_probe(const char *buf, off_t len)
> > {
> > - struct x86_linux_header header;
> > + const struct x86_linux_header *header;
> > if ((uintmax_t)len < (uintmax_t)sizeof(header)) {
> > return -1;
> > }
> > - memcpy(&header, buf, sizeof(header));
> > - if (memcmp(header.header_magic, "HdrS", 4) != 0) {
> > + header = (void *)buf;
>
> Perhaps casting to (struct x86_linux_header*) would read better?
>
I felt that '(const struct x86_linux_header *)' is a bit long, but it's
not a big deal. Attached is the same patch with that line modified:
==>
Don't wholeheartedly copy the 32-Kbytes bzImage header area to the stack,
just use a constant pointer instead.
Signed-off-by: Ahmed S. Darwish <darwish.07@gmail.com>
---
diff --git a/kexec/arch/i386/kexec-bzImage.c b/kexec/arch/i386/kexec-bzImage.c
index 83d3a69..29e9165 100644
--- a/kexec/arch/i386/kexec-bzImage.c
+++ b/kexec/arch/i386/kexec-bzImage.c
@@ -43,32 +43,32 @@ static const int probe_debug = 0;
int bzImage_probe(const char *buf, off_t len)
{
- struct x86_linux_header header;
+ const struct x86_linux_header *header;
if ((uintmax_t)len < (uintmax_t)sizeof(header)) {
return -1;
}
- memcpy(&header, buf, sizeof(header));
- if (memcmp(header.header_magic, "HdrS", 4) != 0) {
+ header = (const struct x86_linux_header *)buf;
+ if (memcmp(header->header_magic, "HdrS", 4) != 0) {
if (probe_debug) {
fprintf(stderr, "Not a bzImage\n");
}
return -1;
}
- if (header.boot_sector_magic != 0xAA55) {
+ if (header->boot_sector_magic != 0xAA55) {
if (probe_debug) {
fprintf(stderr, "No x86 boot sector present\n");
}
/* No x86 boot sector present */
return -1;
}
- if (header.protocol_version < 0x0200) {
+ if (header->protocol_version < 0x0200) {
if (probe_debug) {
fprintf(stderr, "Must be at least protocol version 2.00\n");
}
/* Must be at least protocol version 2.00 */
return -1;
}
- if ((header.loadflags & 1) == 0) {
+ if ((header->loadflags & 1) == 0) {
if (probe_debug) {
fprintf(stderr, "zImage not a bzImage\n");
}
thanks,
--
Darwish
http://darwish.07.googlepages.com
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] kexec-tools: Don't duplicate the bzImage header area
2011-02-09 11:13 ` [PATCH v2] " Ahmed S. Darwish
@ 2011-02-09 23:25 ` Simon Horman
0 siblings, 0 replies; 4+ messages in thread
From: Simon Horman @ 2011-02-09 23:25 UTC (permalink / raw)
To: Ahmed S. Darwish; +Cc: Haren Myneni, kexec, Eric Biederman, Vivek Goyal
On Wed, Feb 09, 2011 at 01:13:04PM +0200, Ahmed S. Darwish wrote:
> On Wed, Feb 09, 2011 at 07:15:45AM +0900, Simon Horman wrote:
> > On Tue, Feb 08, 2011 at 05:20:36PM +0200, Ahmed S. Darwish wrote:
> > >
> > > diff --git a/kexec/arch/i386/kexec-bzImage.c b/kexec/arch/i386/kexec-bzImage.c
> > > index 83d3a69..2312eb8 100644
> > > --- a/kexec/arch/i386/kexec-bzImage.c
> > > +++ b/kexec/arch/i386/kexec-bzImage.c
> > > @@ -43,32 +43,32 @@ static const int probe_debug = 0;
> > >
> > > int bzImage_probe(const char *buf, off_t len)
> > > {
> > > - struct x86_linux_header header;
> > > + const struct x86_linux_header *header;
> > > if ((uintmax_t)len < (uintmax_t)sizeof(header)) {
> > > return -1;
> > > }
> > > - memcpy(&header, buf, sizeof(header));
> > > - if (memcmp(header.header_magic, "HdrS", 4) != 0) {
> > > + header = (void *)buf;
> >
> > Perhaps casting to (struct x86_linux_header*) would read better?
> >
>
> I felt that '(const struct x86_linux_header *)' is a bit long, but it's
> not a big deal. Attached is the same patch with that line modified:
>
> ==>
>
> Don't wholeheartedly copy the 32-Kbytes bzImage header area to the stack,
> just use a constant pointer instead.
Thanks, applied.
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2011-02-09 23:25 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-02-08 15:20 [PATCH] kexec-tools: Don't duplicate the bzImage header area Ahmed S. Darwish
2011-02-08 22:15 ` Simon Horman
2011-02-09 11:13 ` [PATCH v2] " Ahmed S. Darwish
2011-02-09 23:25 ` Simon Horman
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.