linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Steven Rostedt <rostedt@goodmis.org>
To: linux-kernel@vger.kernel.org
Cc: Ingo Molnar <mingo@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Matt Helsley <mhelsley@vmware.com>
Subject: [for-next][PATCH 20/25] recordmcount: Remove redundant cleanup() calls
Date: Thu, 05 Sep 2019 11:43:18 -0400	[thread overview]
Message-ID: <20190905154343.118751641@goodmis.org> (raw)
In-Reply-To: 20190905154258.573706229@goodmis.org

From: Matt Helsley <mhelsley@vmware.com>

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.com

Signed-off-by: Matt Helsley <mhelsley@vmware.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 scripts/recordmcount.c | 13 -------------
 scripts/recordmcount.h |  2 --
 2 files changed, 15 deletions(-)

diff --git a/scripts/recordmcount.c b/scripts/recordmcount.c
index 273ca8b42b20..5677fcc88a72 100644
--- a/scripts/recordmcount.c
+++ b/scripts/recordmcount.c
@@ -258,17 +258,14 @@ static void *mmap_file(char const *fname)
 	fd_map = open(fname, O_RDONLY);
 	if (fd_map < 0) {
 		perror(fname);
-		cleanup();
 		return NULL;
 	}
 	if (fstat(fd_map, &sb) < 0) {
 		perror(fname);
-		cleanup();
 		goto out;
 	}
 	if (!S_ISREG(sb.st_mode)) {
 		fprintf(stderr, "not a regular file: %s\n", fname);
-		cleanup();
 		goto out;
 	}
 	file_map = mmap(0, sb.st_size, PROT_READ|PROT_WRITE, MAP_PRIVATE,
@@ -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);
 	if (fd_map < 0) {
 		perror(fname);
-		cleanup();
 		return -1;
 	}
 	n = write(fd_map, file_map, sb.st_size);
 	if (n != sb.st_size) {
 		perror("write");
-		cleanup();
 		close(fd_map);
 		return -1;
 	}
@@ -328,7 +323,6 @@ static int write_file(const char *fname)
 		n = write(fd_map, file_append, file_append_size);
 		if (n != file_append_size) {
 			perror("write");
-			cleanup();
 			close(fd_map);
 			return -1;
 		}
@@ -336,7 +330,6 @@ static int write_file(const char *fname)
 	close(fd_map);
 	if (rename(tmp_file, fname) < 0) {
 		perror(fname);
-		cleanup();
 		return -1;
 	}
 	return 0;
@@ -460,7 +453,6 @@ static int do_file(char const *const fname)
 	default:
 		fprintf(stderr, "unrecognized ELF data encoding %d: %s\n",
 			ehdr->e_ident[EI_DATA], fname);
-		cleanup();
 		goto out;
 	case ELFDATA2LSB:
 		if (*(unsigned char const *)&endian != 1) {
@@ -493,7 +485,6 @@ static int do_file(char const *const fname)
 	    w2(ehdr->e_type) != ET_REL ||
 	    ehdr->e_ident[EI_VERSION] != EV_CURRENT) {
 		fprintf(stderr, "unrecognized ET_REL file %s\n", fname);
-		cleanup();
 		goto out;
 	}
 
@@ -502,7 +493,6 @@ static int do_file(char const *const fname)
 	default:
 		fprintf(stderr, "unrecognized e_machine %u %s\n",
 			w2(ehdr->e_machine), fname);
-		cleanup();
 		goto out;
 	case EM_386:
 		reltype = R_386_32;
@@ -546,14 +536,12 @@ static int do_file(char const *const fname)
 	default:
 		fprintf(stderr, "unrecognized ELF class %d %s\n",
 			ehdr->e_ident[EI_CLASS], fname);
-		cleanup();
 		goto out;
 	case ELFCLASS32:
 		if (w2(ehdr->e_ehsize) != sizeof(Elf32_Ehdr)
 		||  w2(ehdr->e_shentsize) != sizeof(Elf32_Shdr)) {
 			fprintf(stderr,
 				"unrecognized ET_REL file: %s\n", fname);
-			cleanup();
 			goto out;
 		}
 		if (w2(ehdr->e_machine) == EM_MIPS) {
@@ -569,7 +557,6 @@ static int do_file(char const *const fname)
 		||  w2(ghdr->e_shentsize) != sizeof(Elf64_Shdr)) {
 			fprintf(stderr,
 				"unrecognized ET_REL file: %s\n", fname);
-			cleanup();
 			goto out;
 		}
 		if (w2(ghdr->e_machine) == EM_S390) {
diff --git a/scripts/recordmcount.h b/scripts/recordmcount.h
index ca9aaac89bfb..8f0a278ce0af 100644
--- a/scripts/recordmcount.h
+++ b/scripts/recordmcount.h
@@ -463,7 +463,6 @@ static int find_secsym_ndx(unsigned const txtndx,
 	}
 	fprintf(stderr, "Cannot find symbol for section %u: %s.\n",
 		txtndx, txtname);
-	cleanup();
 	return -1;
 }
 
@@ -480,7 +479,6 @@ static char const * __has_rel_mcount(Elf_Shdr const *const relhdr, /* reltype */
 	if (strcmp("__mcount_loc", txtname) == 0) {
 		fprintf(stderr, "warning: __mcount_loc already exists: %s\n",
 			fname);
-		cleanup();
 		return already_has_rel_mcount;
 	}
 	if (w(txthdr->sh_type) != SHT_PROGBITS ||
-- 
2.20.1



  parent reply	other threads:[~2019-09-05 15:44 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-05 15:42 [for-next][PATCH 00/25] tracing: Updates for 5.4 Steven Rostedt
2019-09-05 15:42 ` [for-next][PATCH 01/25] kprobes: Allow kprobes coexist with livepatch Steven Rostedt
2019-09-05 15:43 ` [for-next][PATCH 02/25] recordmcount: Remove redundant strcmp Steven Rostedt
2019-09-05 15:43 ` [for-next][PATCH 03/25] recordmcount: Remove uread() Steven Rostedt
2019-09-05 15:43 ` [for-next][PATCH 04/25] recordmcount: Remove unused fd from uwrite() and ulseek() Steven Rostedt
2019-09-05 15:43 ` [for-next][PATCH 05/25] tracing/probe: Split trace_event related data from trace_probe Steven Rostedt
2019-09-05 15:43 ` [for-next][PATCH 06/25] tracing/dynevent: Delete all matched events Steven Rostedt
2019-09-05 15:43 ` [for-next][PATCH 07/25] tracing/dynevent: Pass extra arguments to match operation Steven Rostedt
2019-09-05 15:43 ` [for-next][PATCH 08/25] tracing/kprobe: Add multi-probe per event support Steven Rostedt
2019-09-05 15:43 ` [for-next][PATCH 09/25] tracing/uprobe: Add multi-probe per uprobe " Steven Rostedt
2019-09-05 15:43 ` [for-next][PATCH 10/25] tracing/kprobe: Add per-probe delete from event Steven Rostedt
2019-09-05 15:43 ` [for-next][PATCH 11/25] tracing/uprobe: " Steven Rostedt
2019-09-05 15:43 ` [for-next][PATCH 12/25] tracing/probe: Add immediate parameter support Steven Rostedt
2019-09-05 15:43 ` [for-next][PATCH 13/25] tracing/probe: Add immediate string " Steven Rostedt
2019-09-05 15:43 ` [for-next][PATCH 14/25] selftests/ftrace: Add a testcase for kprobe multiprobe event Steven Rostedt
2019-09-05 15:43 ` [for-next][PATCH 15/25] selftests/ftrace: Add syntax error test for immediates Steven Rostedt
2019-09-05 15:43 ` [for-next][PATCH 16/25] selftests/ftrace: Add syntax error test for multiprobe Steven Rostedt
2019-09-05 15:43 ` [for-next][PATCH 17/25] recordmcount: Rewrite error/success handling Steven Rostedt
2019-09-05 15:43 ` [for-next][PATCH 18/25] recordmcount: Kernel style function signature formatting Steven Rostedt
2019-09-05 15:43 ` [for-next][PATCH 19/25] recordmcount: Kernel style formatting Steven Rostedt
2019-09-05 15:43 ` Steven Rostedt [this message]
2019-09-05 15:43 ` [for-next][PATCH 21/25] recordmcount: Clarify what cleanup() does Steven Rostedt
2019-09-05 15:43 ` [for-next][PATCH 22/25] tracing/arm64: Have max stack tracer handle the case of return address after data Steven Rostedt
2019-09-05 15:43 ` [for-next][PATCH 23/25] tracing: Document the stack trace algorithm in the comments Steven Rostedt
2019-09-05 15:43 ` [for-next][PATCH 24/25] tracing: Rename tracing_reset() to tracing_reset_cpu() Steven Rostedt
2019-09-05 15:43 ` [for-next][PATCH 25/25] tracing: Add "gfp_t" support in synthetic_events Steven Rostedt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190905154343.118751641@goodmis.org \
    --to=rostedt@goodmis.org \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mhelsley@vmware.com \
    --cc=mingo@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).