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

Message ID 20180711133709.26788-1-y.karadz@gmail.com
State New, archived
Headers show
Series
  • [1/2] kernel-shark-qt: Deal with the "no tasks" case in kshark_get_task_pids()
Related show

Commit Message

Yordan Karadzhov (VMware) July 11, 2018, 1:37 p.m. UTC
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(+)

Comments

Steven Rostedt July 11, 2018, 2:02 p.m. UTC | #1
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
Yordan Karadzhov (VMware) July 11, 2018, 2:08 p.m. UTC | #2
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
>
Yordan Karadzhov (VMware) Aug. 3, 2018, 1:06 p.m. UTC | #3
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
Steven Rostedt Aug. 3, 2018, 5:46 p.m. UTC | #4
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
Steven Rostedt Aug. 3, 2018, 6:18 p.m. UTC | #5
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
Steven Rostedt Aug. 3, 2018, 6:22 p.m. UTC | #6
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

Patch
diff mbox series

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;