[v3,5/7] trace-cmd: Refactored make_instances() and tracecmd_remove_instances()
diff mbox series

Message ID 20190208134918.5618-6-tstoyanov@vmware.com
State Superseded
Headers show
Series
  • trace-cmd: Timetamps sync using PTP-like algorithm, relying on vsock events.
Related show

Commit Message

Tzvetomir Stoyanov Feb. 8, 2019, 1:49 p.m. UTC
In order to reuse the code which creates / deletes tracing instances,
these two functions are refactotred. A new ones are implemented:
make_one_instances() and tracecmd_remove_one_instance().

Signed-off-by: Tzvetomir Stoyanov <tstoyanov@vmware.com>
---
 tracecmd/trace-record.c | 54 +++++++++++++++++++++++------------------
 1 file changed, 31 insertions(+), 23 deletions(-)

Comments

Steven Rostedt Feb. 8, 2019, 10:11 p.m. UTC | #1
On Fri,  8 Feb 2019 15:49:16 +0200
Tzvetomir Stoyanov <tstoyanov@vmware.com> wrote:

> In order to reuse the code which creates / deletes tracing instances,
> these two functions are refactotred. A new ones are implemented:
> make_one_instances() and tracecmd_remove_one_instance().
> 

Note, I still prefer just dropping the "s":

	make_instances() (more than one)
	make_instance() (just one)

-- Steve
Steven Rostedt Feb. 8, 2019, 10:44 p.m. UTC | #2
On Fri,  8 Feb 2019 15:49:16 +0200
Tzvetomir Stoyanov <tstoyanov@vmware.com> wrote:

> +static void tracecmd_remove_one_instance(struct buffer_instance *instance)
> +{
> +	char *path;
> +
> +	if (instance->tracing_on_fd > 0) {
> +		close(instance->tracing_on_fd);
> +		instance->tracing_on_fd = 0;

Also note that uninitialized file descriptors should be a negative
number (-1), because zero is a valid descriptor value.

-- Steve

>  	}
> +	path = get_instance_dir(instance);
> +	tracecmd_put_tracing_file(path);
>  }
>
Steven Rostedt Feb. 8, 2019, 10:46 p.m. UTC | #3
On Fri, 8 Feb 2019 17:44:09 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Fri,  8 Feb 2019 15:49:16 +0200
> Tzvetomir Stoyanov <tstoyanov@vmware.com> wrote:
> 
> > +static void tracecmd_remove_one_instance(struct buffer_instance *instance)
> > +{
> > +	char *path;
> > +
> > +	if (instance->tracing_on_fd > 0) {
> > +		close(instance->tracing_on_fd);
> > +		instance->tracing_on_fd = 0;  
> 
> Also note that uninitialized file descriptors should be a negative
> number (-1), because zero is a valid descriptor value.

Bah, nevermind. Looks like I was a bit more lenient back in the day...

d56f30679 trace-record.c          (Steven Rostedt (Red Hat)    2013-03-06 11:31:39 -0500 1938)  /* OK, we keep zero for stdin */
d56f30679 trace-record.c          (Steven Rostedt (Red Hat)    2013-03-06 11:31:39 -0500 1939)  if (fd > 0)
6e7c1dd76 trace-record.c          (Steven Rostedt              2011-02-23 13:28:04 -0500 1940)          return fd;


  ;-)

-- Steve

Patch
diff mbox series

diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
index 01b360a..e1ad797 100644
--- a/tracecmd/trace-record.c
+++ b/tracecmd/trace-record.c
@@ -4304,49 +4304,57 @@  static void clear_func_filters(void)
 	}
 }
 
-static void make_instances(void)
+static void make_one_instance(struct buffer_instance *instance)
 {
-	struct buffer_instance *instance;
 	struct stat st;
 	char *path;
 	int ret;
 
+	path = get_instance_dir(instance);
+	ret = stat(path, &st);
+	if (ret < 0) {
+		ret = mkdir(path, 0777);
+		if (ret < 0)
+			die("mkdir %s", path);
+	} else
+		/* Don't delete instances that already exist */
+		instance->flags |= BUFFER_FL_KEEP;
+	tracecmd_put_tracing_file(path);
+
+}
+
+static void make_instances(void)
+{
+	struct buffer_instance *instance;
+
 	for_each_instance(instance) {
 		if (instance->flags & BUFFER_FL_GUEST)
 			continue;
+		make_one_instance(instance);
+	}
+}
 
-		path = get_instance_dir(instance);
-		ret = stat(path, &st);
-		if (ret < 0) {
-			ret = mkdir(path, 0777);
-			if (ret < 0)
-				die("mkdir %s", path);
-		} else
-			/* Don't delete instances that already exist */
-			instance->flags |= BUFFER_FL_KEEP;
-		tracecmd_put_tracing_file(path);
+static void tracecmd_remove_one_instance(struct buffer_instance *instance)
+{
+	char *path;
+
+	if (instance->tracing_on_fd > 0) {
+		close(instance->tracing_on_fd);
+		instance->tracing_on_fd = 0;
 	}
+	path = get_instance_dir(instance);
+	tracecmd_put_tracing_file(path);
 }
 
 void tracecmd_remove_instances(void)
 {
 	struct buffer_instance *instance;
-	char *path;
-	int ret;
 
 	for_each_instance(instance) {
 		/* Only delete what we created */
 		if (instance->flags & (BUFFER_FL_KEEP | BUFFER_FL_GUEST))
 			continue;
-		if (instance->tracing_on_fd > 0) {
-			close(instance->tracing_on_fd);
-			instance->tracing_on_fd = 0;
-		}
-		path = get_instance_dir(instance);
-		ret = rmdir(path);
-		if (ret < 0)
-			die("rmdir %s", path);
-		tracecmd_put_tracing_file(path);
+		tracecmd_remove_one_instance(instance);
 	}
 }