[PATCH 1/2] kernel-shark-qt: Deal with the "no tasks" case in kshark_get_task_pids()

[PATCH 1/2] kernel-shark-qt: Deal with the "no tasks" case in kshark_get_task_pids()

[Yordan Karadzhov (VMware)]

kshark_get_task_pids() should not return a "memory allocation" error
in the case when it is called before having trace data loaded.

Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
---
 kernel-shark-qt/src/libkshark.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/kernel-shark-qt/src/libkshark.c b/kernel-shark-qt/src/libkshark.c
index 668b6df..75b88c9 100644
--- a/kernel-shark-qt/src/libkshark.c
+++ b/kernel-shark-qt/src/libkshark.c
@@ -303,6 +303,12 @@ ssize_t kshark_get_task_pids(struct kshark_context *kshark_ctx, int **pids)
 		}
 	}
 
+	if (!pid_count) {
+		free(*pids);
+		*pids = NULL;
+		return pid_count;
+	}
+
 	temp_pids = realloc(*pids, pid_count * sizeof(int));
 	if (!temp_pids)
 		goto fail;
-- 
2.17.1

[PATCH 2/2] kernel-shark-qt: Fix the default value of the Event Visible flag

[Yordan Karadzhov (VMware)]

This patch fixes a simpel bug in the way the "Event Visible" flag of the
kshark_entries is set when the data is loadded.

Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
---
 kernel-shark-qt/src/libkshark.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel-shark-qt/src/libkshark.c b/kernel-shark-qt/src/libkshark.c
index 75b88c9..3299752 100644
--- a/kernel-shark-qt/src/libkshark.c
+++ b/kernel-shark-qt/src/libkshark.c
@@ -600,7 +600,7 @@ static size_t get_records(struct kshark_context *kshark_ctx,
 				kshark_set_entry_values(kshark_ctx, rec, entry);
 				pid = entry->pid;
 				/* Apply event filtering. */
-				ret = FILTER_NONE;
+				ret = FILTER_MATCH;
 				if (adv_filter->filters)
 					ret = pevent_filter_match(adv_filter, rec);
 
-- 
2.17.1

Re: [PATCH 2/2] kernel-shark-qt: Fix the default value of the Event Visible flag

[Steven Rostedt]

On Wed, 11 Jul 2018 16:37:09 +0300
"Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:

> This patch fixes a simpel bug in the way the "Event Visible" flag of the
> kshark_entries is set when the data is loadded.
> 
> Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
> ---

Applied. Thanks Yordan!

-- Steve

Re: [PATCH 1/2] kernel-shark-qt: Deal with the "no tasks" case in kshark_get_task_pids()

[Steven Rostedt]

On Wed, 11 Jul 2018 16:37:08 +0300
"Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:

> kshark_get_task_pids() should not return a "memory allocation" error
> in the case when it is called before having trace data loaded.
> 
> Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
> ---
>  kernel-shark-qt/src/libkshark.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/kernel-shark-qt/src/libkshark.c b/kernel-shark-qt/src/libkshark.c
> index 668b6df..75b88c9 100644
> --- a/kernel-shark-qt/src/libkshark.c
> +++ b/kernel-shark-qt/src/libkshark.c
> @@ -303,6 +303,12 @@ ssize_t kshark_get_task_pids(struct kshark_context *kshark_ctx, int **pids)
>  		}
>  	}
>  
> +	if (!pid_count) {
> +		free(*pids);
> +		*pids = NULL;
> +		return pid_count;
> +	}
> +
>  	temp_pids = realloc(*pids, pid_count * sizeof(int));
>  	if (!temp_pids)
>  		goto fail;

What about doing it this way:

diff --git a/kernel-shark-qt/src/libkshark.c b/kernel-shark-qt/src/libkshark.c
index 668b6df8..34daa25e 100644
--- a/kernel-shark-qt/src/libkshark.c
+++ b/kernel-shark-qt/src/libkshark.c
@@ -303,12 +303,17 @@ ssize_t kshark_get_task_pids(struct kshark_context *kshark_ctx, int **pids)
 		}
 	}
 
-	temp_pids = realloc(*pids, pid_count * sizeof(int));
-	if (!temp_pids)
-		goto fail;
+	if (pid_count) {
+		temp_pids = realloc(*pids, pid_count * sizeof(int));
+		if (!temp_pids)
+			goto fail;
 
-	/* Paranoid: In the unlikely case of shrinking *pids, realloc moves it */
-	*pids = temp_pids;
+		/* Paranoid: In the unlikely case of shrinking *pids, realloc moves it */
+		*pids = temp_pids;
+	} else {
+		free(*pids);
+		*pids = NULL;
+	}
 
 	return pid_count;
 

?

-- Steve

Re: [PATCH 1/2] kernel-shark-qt: Deal with the "no tasks" case in kshark_get_task_pids()

[Yordan Karadzhov (VMware)]

On 11.07.2018 17:02, Steven Rostedt wrote:
> On Wed, 11 Jul 2018 16:37:08 +0300
> "Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:
> 
>> kshark_get_task_pids() should not return a "memory allocation" error
>> in the case when it is called before having trace data loaded.
>>
>> Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
>> ---
>>   kernel-shark-qt/src/libkshark.c | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/kernel-shark-qt/src/libkshark.c b/kernel-shark-qt/src/libkshark.c
>> index 668b6df..75b88c9 100644
>> --- a/kernel-shark-qt/src/libkshark.c
>> +++ b/kernel-shark-qt/src/libkshark.c
>> @@ -303,6 +303,12 @@ ssize_t kshark_get_task_pids(struct kshark_context *kshark_ctx, int **pids)
>>   		}
>>   	}
>>   
>> +	if (!pid_count) {
>> +		free(*pids);
>> +		*pids = NULL;
>> +		return pid_count;
>> +	}
>> +
>>   	temp_pids = realloc(*pids, pid_count * sizeof(int));
>>   	if (!temp_pids)
>>   		goto fail;
> 
> What about doing it this way:
> 
> diff --git a/kernel-shark-qt/src/libkshark.c b/kernel-shark-qt/src/libkshark.c
> index 668b6df8..34daa25e 100644
> --- a/kernel-shark-qt/src/libkshark.c
> +++ b/kernel-shark-qt/src/libkshark.c
> @@ -303,12 +303,17 @@ ssize_t kshark_get_task_pids(struct kshark_context *kshark_ctx, int **pids)
>   		}
>   	}
>   
> -	temp_pids = realloc(*pids, pid_count * sizeof(int));
> -	if (!temp_pids)
> -		goto fail;
> +	if (pid_count) {
> +		temp_pids = realloc(*pids, pid_count * sizeof(int));
> +		if (!temp_pids)
> +			goto fail;
>   
> -	/* Paranoid: In the unlikely case of shrinking *pids, realloc moves it */
> -	*pids = temp_pids;
> +		/* Paranoid: In the unlikely case of shrinking *pids, realloc moves it */
> +		*pids = temp_pids;
> +	} else {
> +		free(*pids);
> +		*pids = NULL;
> +	}
>   
>   	return pid_count;
>   

Yes, this way is better.

Thanks!
Yordan


> 
> ?
> 
> -- Steve
> 

Re: [PATCH 1/2] kernel-shark-qt: Deal with the "no tasks" case in kshark_get_task_pids()

[Yordan Karadzhov (VMware)]

Hi Steven,
Should I send this patch again?

Thanks!
Yordan


On 11.07.2018 17:08, Yordan Karadzhov (VMware) wrote:
>>
>> What about doing it this way:
>>
>> diff --git a/kernel-shark-qt/src/libkshark.c 
>> b/kernel-shark-qt/src/libkshark.c
>> index 668b6df8..34daa25e 100644
>> --- a/kernel-shark-qt/src/libkshark.c
>> +++ b/kernel-shark-qt/src/libkshark.c
>> @@ -303,12 +303,17 @@ ssize_t kshark_get_task_pids(struct 
>> kshark_context *kshark_ctx, int **pids)
>>           }
>>       }
>> -    temp_pids = realloc(*pids, pid_count * sizeof(int));
>> -    if (!temp_pids)
>> -        goto fail;
>> +    if (pid_count) {
>> +        temp_pids = realloc(*pids, pid_count * sizeof(int));
>> +        if (!temp_pids)
>> +            goto fail;
>> -    /* Paranoid: In the unlikely case of shrinking *pids, realloc 
>> moves it */
>> -    *pids = temp_pids;
>> +        /* Paranoid: In the unlikely case of shrinking *pids, realloc 
>> moves it */
>> +        *pids = temp_pids;
>> +    } else {
>> +        free(*pids);
>> +        *pids = NULL;
>> +    }
>>       return pid_count;
> 
> Yes, this way is better.
> 
> Thanks!
> Yordan
> 
> 
>>
>> ?
>>
>> -- Steve

Re: [PATCH 1/2] kernel-shark-qt: Deal with the "no tasks" case in kshark_get_task_pids()

[Steven Rostedt]

On Fri, 3 Aug 2018 16:06:24 +0300
"Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:

> Hi Steven,
> Should I send this patch again?

Yes, with the updates.

Thanks!

-- Steve

Re: [PATCH 1/2] kernel-shark-qt: Deal with the "no tasks" case in kshark_get_task_pids()

[Steven Rostedt]

On Fri, 3 Aug 2018 13:46:22 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Fri, 3 Aug 2018 16:06:24 +0300
> "Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:
> 
> > Hi Steven,
> > Should I send this patch again?  
> 
> Yes, with the updates.
> 

I take this back. I'll do the updates and apply your other patches on
top.

Thanks!

-- Steve

Re: [PATCH 1/2] kernel-shark-qt: Deal with the "no tasks" case in kshark_get_task_pids()

[Steven Rostedt]

On Fri, 3 Aug 2018 14:18:27 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Fri, 3 Aug 2018 13:46:22 -0400
> Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> > On Fri, 3 Aug 2018 16:06:24 +0300
> > "Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:
> >   
> > > Hi Steven,
> > > Should I send this patch again?    
> > 
> > Yes, with the updates.
> >   
> 
> I take this back. I'll do the updates and apply your other patches on
> top.
> 

And this is what I get when I work on too many projects at once. I
already have the update applied to my local tree :-p.

-- Steve