Commit c97fea26 authored by Matt Helsley's avatar Matt Helsley Committed by Steven Rostedt (VMware)

recordmcount: Remove redundant cleanup() calls

Redundant cleanup calls were introduced when transitioning from
the old error/success handling via setjmp/longjmp -- the longjmp
ensured the cleanup() call only happened once but replacing
the success_file()/fail_file() calls with cleanup() meant that
multiple cleanup() calls can happen as we return from function
calls.

In do_file(), looking just before and after the "goto out" jumps we
can see that multiple cleanups() are being performed. We remove
cleanup() calls from the nested functions because it makes the code
easier to review -- the resources being cleaned up are generally
allocated and initialized in the callers so freeing them there
makes more sense.

Other redundant cleanup() calls:

mmap_file() is only called from do_file() and, if mmap_file() fails,
then we goto out and do cleanup() there too.

write_file() is only called from do_file() and do_file()
calls cleanup() unconditionally after returning from write_file()
therefore the cleanup() calls in write_file() are not necessary.

find_secsym_ndx(), called from do_func()'s for-loop, when we are
cleaning up here it's obvious that we break out of the loop and
do another cleanup().

__has_rel_mcount() is called from two parts of do_func()
and calls cleanup(). In theory we move them into do_func(), however
these in turn prove redundant so another simplification step
removes them as well.

Link: http://lkml.kernel.org/r/de197e17fc5426623a847ea7cf3a1560a7402a4b.1564596289.git.mhelsley@vmware.comSigned-off-by: default avatarMatt Helsley <mhelsley@vmware.com>
Signed-off-by: default avatarSteven Rostedt (VMware) <rostedt@goodmis.org>
parent 2e63152b
...@@ -258,17 +258,14 @@ static void *mmap_file(char const *fname) ...@@ -258,17 +258,14 @@ static void *mmap_file(char const *fname)
fd_map = open(fname, O_RDONLY); fd_map = open(fname, O_RDONLY);
if (fd_map < 0) { if (fd_map < 0) {
perror(fname); perror(fname);
cleanup();
return NULL; return NULL;
} }
if (fstat(fd_map, &sb) < 0) { if (fstat(fd_map, &sb) < 0) {
perror(fname); perror(fname);
cleanup();
goto out; goto out;
} }
if (!S_ISREG(sb.st_mode)) { if (!S_ISREG(sb.st_mode)) {
fprintf(stderr, "not a regular file: %s\n", fname); fprintf(stderr, "not a regular file: %s\n", fname);
cleanup();
goto out; goto out;
} }
file_map = mmap(0, sb.st_size, PROT_READ|PROT_WRITE, MAP_PRIVATE, file_map = mmap(0, sb.st_size, PROT_READ|PROT_WRITE, MAP_PRIVATE,
...@@ -314,13 +311,11 @@ static int write_file(const char *fname) ...@@ -314,13 +311,11 @@ static int write_file(const char *fname)
fd_map = open(tmp_file, O_WRONLY | O_TRUNC | O_CREAT, sb.st_mode); fd_map = open(tmp_file, O_WRONLY | O_TRUNC | O_CREAT, sb.st_mode);
if (fd_map < 0) { if (fd_map < 0) {
perror(fname); perror(fname);
cleanup();
return -1; return -1;
} }
n = write(fd_map, file_map, sb.st_size); n = write(fd_map, file_map, sb.st_size);
if (n != sb.st_size) { if (n != sb.st_size) {
perror("write"); perror("write");
cleanup();
close(fd_map); close(fd_map);
return -1; return -1;
} }
...@@ -328,7 +323,6 @@ static int write_file(const char *fname) ...@@ -328,7 +323,6 @@ static int write_file(const char *fname)
n = write(fd_map, file_append, file_append_size); n = write(fd_map, file_append, file_append_size);
if (n != file_append_size) { if (n != file_append_size) {
perror("write"); perror("write");
cleanup();
close(fd_map); close(fd_map);
return -1; return -1;
} }
...@@ -336,7 +330,6 @@ static int write_file(const char *fname) ...@@ -336,7 +330,6 @@ static int write_file(const char *fname)
close(fd_map); close(fd_map);
if (rename(tmp_file, fname) < 0) { if (rename(tmp_file, fname) < 0) {
perror(fname); perror(fname);
cleanup();
return -1; return -1;
} }
return 0; return 0;
...@@ -460,7 +453,6 @@ static int do_file(char const *const fname) ...@@ -460,7 +453,6 @@ static int do_file(char const *const fname)
default: default:
fprintf(stderr, "unrecognized ELF data encoding %d: %s\n", fprintf(stderr, "unrecognized ELF data encoding %d: %s\n",
ehdr->e_ident[EI_DATA], fname); ehdr->e_ident[EI_DATA], fname);
cleanup();
goto out; goto out;
case ELFDATA2LSB: case ELFDATA2LSB:
if (*(unsigned char const *)&endian != 1) { if (*(unsigned char const *)&endian != 1) {
...@@ -493,7 +485,6 @@ static int do_file(char const *const fname) ...@@ -493,7 +485,6 @@ static int do_file(char const *const fname)
w2(ehdr->e_type) != ET_REL || w2(ehdr->e_type) != ET_REL ||
ehdr->e_ident[EI_VERSION] != EV_CURRENT) { ehdr->e_ident[EI_VERSION] != EV_CURRENT) {
fprintf(stderr, "unrecognized ET_REL file %s\n", fname); fprintf(stderr, "unrecognized ET_REL file %s\n", fname);
cleanup();
goto out; goto out;
} }
...@@ -502,7 +493,6 @@ static int do_file(char const *const fname) ...@@ -502,7 +493,6 @@ static int do_file(char const *const fname)
default: default:
fprintf(stderr, "unrecognized e_machine %u %s\n", fprintf(stderr, "unrecognized e_machine %u %s\n",
w2(ehdr->e_machine), fname); w2(ehdr->e_machine), fname);
cleanup();
goto out; goto out;
case EM_386: case EM_386:
reltype = R_386_32; reltype = R_386_32;
...@@ -546,14 +536,12 @@ static int do_file(char const *const fname) ...@@ -546,14 +536,12 @@ static int do_file(char const *const fname)
default: default:
fprintf(stderr, "unrecognized ELF class %d %s\n", fprintf(stderr, "unrecognized ELF class %d %s\n",
ehdr->e_ident[EI_CLASS], fname); ehdr->e_ident[EI_CLASS], fname);
cleanup();
goto out; goto out;
case ELFCLASS32: case ELFCLASS32:
if (w2(ehdr->e_ehsize) != sizeof(Elf32_Ehdr) if (w2(ehdr->e_ehsize) != sizeof(Elf32_Ehdr)
|| w2(ehdr->e_shentsize) != sizeof(Elf32_Shdr)) { || w2(ehdr->e_shentsize) != sizeof(Elf32_Shdr)) {
fprintf(stderr, fprintf(stderr,
"unrecognized ET_REL file: %s\n", fname); "unrecognized ET_REL file: %s\n", fname);
cleanup();
goto out; goto out;
} }
if (w2(ehdr->e_machine) == EM_MIPS) { if (w2(ehdr->e_machine) == EM_MIPS) {
...@@ -569,7 +557,6 @@ static int do_file(char const *const fname) ...@@ -569,7 +557,6 @@ static int do_file(char const *const fname)
|| w2(ghdr->e_shentsize) != sizeof(Elf64_Shdr)) { || w2(ghdr->e_shentsize) != sizeof(Elf64_Shdr)) {
fprintf(stderr, fprintf(stderr,
"unrecognized ET_REL file: %s\n", fname); "unrecognized ET_REL file: %s\n", fname);
cleanup();
goto out; goto out;
} }
if (w2(ghdr->e_machine) == EM_S390) { if (w2(ghdr->e_machine) == EM_S390) {
......
...@@ -463,7 +463,6 @@ static int find_secsym_ndx(unsigned const txtndx, ...@@ -463,7 +463,6 @@ static int find_secsym_ndx(unsigned const txtndx,
} }
fprintf(stderr, "Cannot find symbol for section %u: %s.\n", fprintf(stderr, "Cannot find symbol for section %u: %s.\n",
txtndx, txtname); txtndx, txtname);
cleanup();
return -1; return -1;
} }
...@@ -480,7 +479,6 @@ static char const * __has_rel_mcount(Elf_Shdr const *const relhdr, /* reltype */ ...@@ -480,7 +479,6 @@ static char const * __has_rel_mcount(Elf_Shdr const *const relhdr, /* reltype */
if (strcmp("__mcount_loc", txtname) == 0) { if (strcmp("__mcount_loc", txtname) == 0) {
fprintf(stderr, "warning: __mcount_loc already exists: %s\n", fprintf(stderr, "warning: __mcount_loc already exists: %s\n",
fname); fname);
cleanup();
return already_has_rel_mcount; return already_has_rel_mcount;
} }
if (w(txthdr->sh_type) != SHT_PROGBITS || if (w(txthdr->sh_type) != SHT_PROGBITS ||
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment