| Message ID | 20190215065708.6086-3-andrew@aj.id.au |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | ipmi-hiomap: Tests and fixes for event handling | expand |
Andrew Jeffery <andrew@aj.id.au> writes: > libflash/file.c: In function 'file_erase': > libflash/file.c:134:1: error: the frame size of 4128 bytes is larger than 1024 bytes [-Werror=frame-larger-than=] > } > ^ > > and > > ffspart.c: In function ‘main’: > ffspart.c:529:1: error: the frame size of 4864 bytes is larger than 1024 bytes [-Werror=frame-larger-than=] > } > ^ > > In both cases, mark the local variables as static to avoid the stack. If I was to bikeshed I'd say we could malloc() instead, but this way seems otherwise fine. > The static approach is valid for file.c as the buffer is always filled > with `~0`. Given it's now going to be in .bss due to static we have to > still perform the memset(), but racing memset()s in this fashion won't > be harmful, just wasteful. Part of me hurts reading it, the other part is "meh, fair enough". I'm guessing this is because when we build from skiboot's makefile we end up with the skiboot CFLAGS which are a lot more restrictive than the CFLAGS in external/ ?
On Mon, 18 Feb 2019, at 11:31, Stewart Smith wrote: > Andrew Jeffery <andrew@aj.id.au> writes: > > libflash/file.c: In function 'file_erase': > > libflash/file.c:134:1: error: the frame size of 4128 bytes is larger than 1024 bytes [-Werror=frame-larger-than=] > > } > > ^ > > > > and > > > > ffspart.c: In function ‘main’: > > ffspart.c:529:1: error: the frame size of 4864 bytes is larger than 1024 bytes [-Werror=frame-larger-than=] > > } > > ^ > > > > In both cases, mark the local variables as static to avoid the stack. > > If I was to bikeshed I'd say we could malloc() instead, but this way > seems otherwise fine. Can't forget to free() if you never malloc() :) > > > The static approach is valid for file.c as the buffer is always filled > > with `~0`. Given it's now going to be in .bss due to static we have to > > still perform the memset(), but racing memset()s in this fashion won't > > be harmful, just wasteful. > > Part of me hurts reading it, the other part is "meh, fair enough". Yeah. It was the least invasive change I could make to fix the issue. Hard to beat adding one word. > > I'm guessing this is because when we build from skiboot's makefile we > end up with the skiboot CFLAGS which are a lot more restrictive than the > CFLAGS in external/ ? I guess so? I didn't dig too deep.
diff --git a/external/ffspart/ffspart.c b/external/ffspart/ffspart.c index bb46a9eaf311..9fc015cf872b 100644 --- a/external/ffspart/ffspart.c +++ b/external/ffspart/ffspart.c @@ -350,7 +350,9 @@ static void print_help(const char *pname) int main(int argc, char *argv[]) { - char *pnor = NULL, *input = NULL, line[MAX_LINE]; + static char line[MAX_LINE]; + + char *pnor = NULL, *input = NULL; bool toc_created = false, bad_input = false, allow_empty = false; uint32_t block_size = 0, block_count = 0; struct ffs_hdr *tocs[MAX_TOCS] = { 0 }; diff --git a/libflash/file.c b/libflash/file.c index 72765b5777f9..d790e29df103 100644 --- a/libflash/file.c +++ b/libflash/file.c @@ -117,7 +117,7 @@ static int file_write(struct blocklevel_device *bl, uint64_t dst, const void *sr */ static int file_erase(struct blocklevel_device *bl, uint64_t dst, uint64_t len) { - char buf[4096]; + static char buf[4096]; int i = 0; int rc;
libflash/file.c: In function 'file_erase': libflash/file.c:134:1: error: the frame size of 4128 bytes is larger than 1024 bytes [-Werror=frame-larger-than=] } ^ and ffspart.c: In function ‘main’: ffspart.c:529:1: error: the frame size of 4864 bytes is larger than 1024 bytes [-Werror=frame-larger-than=] } ^ In both cases, mark the local variables as static to avoid the stack. The static approach is valid for file.c as the buffer is always filled with `~0`. Given it's now going to be in .bss due to static we have to still perform the memset(), but racing memset()s in this fashion won't be harmful, just wasteful. For ffspart.c's main(), there are bigger problems if that needs to be re-entrant. Signed-off-by: Andrew Jeffery <andrew@aj.id.au> --- external/ffspart/ffspart.c | 4 +++- libflash/file.c | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-)