2017-04-26 23:46:38

by Taeung Song

Subject: [PATCH] plugin python: Adjust the handling after PyRun_String() failed

Even though PyRun_String() failed,
just 0 will be returned but we need to return -1
that means error status, so fix it.

Signed-off-by: Taeung Song <[email protected]>
---
plugin_python.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/plugin_python.c b/plugin_python.c
index 2997679..dcfad0f 100644
--- a/plugin_python.c
+++ b/plugin_python.c
@@ -24,7 +24,7 @@ static int load_plugin(struct pevent *pevent, const char *path,
const char *name, void *data)
{
PyObject *globals = data;
- int err;
+ int err, ret = 0;
int len = strlen(path) + strlen(name) + 2;
int nlen = strlen(name) + 1;
char *full = malloc(len);
@@ -50,12 +50,13 @@ static int load_plugin(struct pevent *pevent, const char *path,
if (!res) {
fprintf(stderr, "failed loading %s\n", full);
PyErr_Print();
+ ret = -1;
} else
Py_DECREF(res);

free(load);

- return 0;
+ return ret;
}

int PEVENT_PLUGIN_LOADER(struct pevent *pevent)
--
2.7.4

2017-04-27 02:47:32

by Steven Rostedt

Subject: Re: [PATCH] plugin python: Adjust the handling after PyRun_String() failed

On Thu, 27 Apr 2017 08:46:21 +0900
Taeung Song <[email protected]> wrote:

> Even though PyRun_String() failed,
> just 0 will be returned but we need to return -1
> that means error status, so fix it.
>
> Signed-off-by: Taeung Song <[email protected]>
> ---
> plugin_python.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/plugin_python.c b/plugin_python.c
> index 2997679..dcfad0f 100644
> --- a/plugin_python.c
> +++ b/plugin_python.c
> @@ -24,7 +24,7 @@ static int load_plugin(struct pevent *pevent, const char *path,
> const char *name, void *data)
> {
> PyObject *globals = data;
> - int err;
> + int err, ret = 0;

Hmm, we can either reuse err.

> int len = strlen(path) + strlen(name) + 2;
> int nlen = strlen(name) + 1;
> char *full = malloc(len);
> @@ -50,12 +50,13 @@ static int load_plugin(struct pevent *pevent, const char *path,
> if (!res) {
> fprintf(stderr, "failed loading %s\n", full);
> PyErr_Print();
> + ret = -1;
> } else
> Py_DECREF(res);
>
> free(load);
>
> - return 0;
> + return ret;

or do a: return res ? 0 : -1;

-- Steve

> }
>
> int PEVENT_PLUGIN_LOADER(struct pevent *pevent)

2017-04-27 02:52:26

by Taeung Song

Subject: Re: [PATCH] plugin python: Adjust the handling after PyRun_String() failed

On 04/27/2017 11:47 AM, Steven Rostedt wrote:
> On Thu, 27 Apr 2017 08:46:21 +0900
> Taeung Song <[email protected]> wrote:
>
>> Even though PyRun_String() failed,
>> just 0 will be returned but we need to return -1
>> that means error status, so fix it.
>>
>> Signed-off-by: Taeung Song <[email protected]>
>> ---
>> plugin_python.c | 5 +++--
>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/plugin_python.c b/plugin_python.c
>> index 2997679..dcfad0f 100644
>> --- a/plugin_python.c
>> +++ b/plugin_python.c
>> @@ -24,7 +24,7 @@ static int load_plugin(struct pevent *pevent, const char *path,
>> const char *name, void *data)
>> {
>> PyObject *globals = data;
>> - int err;
>> + int err, ret = 0;
>
> Hmm, we can either reuse err.
>

But return value of asprintf() can be more 1,
when it succeeded.
So I think it is better the below you said.

>> int len = strlen(path) + strlen(name) + 2;
>> int nlen = strlen(name) + 1;
>> char *full = malloc(len);
>> @@ -50,12 +50,13 @@ static int load_plugin(struct pevent *pevent, const char *path,
>> if (!res) {
>> fprintf(stderr, "failed loading %s\n", full);
>> PyErr_Print();
>> + ret = -1;
>> } else
>> Py_DECREF(res);
>>
>> free(load);
>>
>> - return 0;
>> + return ret;
>
> or do a: return res ? 0 : -1;
>
> -- Steve
>

Okey! I'll resent the patch after modifying this!


Thanks,
Taeung

2017-04-27 11:30:52

by Steven Rostedt

Subject: Re: [PATCH] plugin python: Adjust the handling after PyRun_String() failed

On Thu, 27 Apr 2017 11:52:14 +0900
Taeung Song <[email protected]> wrote:

> On 04/27/2017 11:47 AM, Steven Rostedt wrote:
> > On Thu, 27 Apr 2017 08:46:21 +0900
> > Taeung Song <[email protected]> wrote:
> >
> >> Even though PyRun_String() failed,
> >> just 0 will be returned but we need to return -1
> >> that means error status, so fix it.
> >>
> >> Signed-off-by: Taeung Song <[email protected]>
> >> ---
> >> plugin_python.c | 5 +++--
> >> 1 file changed, 3 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/plugin_python.c b/plugin_python.c
> >> index 2997679..dcfad0f 100644
> >> --- a/plugin_python.c
> >> +++ b/plugin_python.c
> >> @@ -24,7 +24,7 @@ static int load_plugin(struct pevent *pevent, const char *path,
> >> const char *name, void *data)
> >> {
> >> PyObject *globals = data;
> >> - int err;
> >> + int err, ret = 0;
> >
> > Hmm, we can either reuse err.
> >
>
> But return value of asprintf() can be more 1,
> when it succeeded.
> So I think it is better the below you said.

What I meant was to simply reset err = 0; and set it if we failed and
returned that. No need to add more variables.

>
> >> int len = strlen(path) + strlen(name) + 2;
> >> int nlen = strlen(name) + 1;
> >> char *full = malloc(len);
> >> @@ -50,12 +50,13 @@ static int load_plugin(struct pevent *pevent, const char *path,
> >> if (!res) {
> >> fprintf(stderr, "failed loading %s\n", full);
> >> PyErr_Print();
> >> + ret = -1;
> >> } else
> >> Py_DECREF(res);
> >>
> >> free(load);
> >>
> >> - return 0;
> >> + return ret;
> >
> > or do a: return res ? 0 : -1;
> >
> > -- Steve
> >
>
> Okey! I'll resent the patch after modifying this!

But this is fine as well.

-- Steve

2017-04-27 12:57:55

by Taeung Song

Subject: Re: [PATCH] plugin python: Adjust the handling after PyRun_String() failed

On 04/27/2017 08:30 PM, Steven Rostedt wrote:
> On Thu, 27 Apr 2017 11:52:14 +0900
> Taeung Song <[email protected]> wrote:
>
>> On 04/27/2017 11:47 AM, Steven Rostedt wrote:
>>> On Thu, 27 Apr 2017 08:46:21 +0900
>>> Taeung Song <[email protected]> wrote:
>>>
>>>> Even though PyRun_String() failed,
>>>> just 0 will be returned but we need to return -1
>>>> that means error status, so fix it.
>>>>
>>>> Signed-off-by: Taeung Song <[email protected]>
>>>> ---
>>>> plugin_python.c | 5 +++--
>>>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/plugin_python.c b/plugin_python.c
>>>> index 2997679..dcfad0f 100644
>>>> --- a/plugin_python.c
>>>> +++ b/plugin_python.c
>>>> @@ -24,7 +24,7 @@ static int load_plugin(struct pevent *pevent, const char *path,
>>>> const char *name, void *data)
>>>> {
>>>> PyObject *globals = data;
>>>> - int err;
>>>> + int err, ret = 0;
>>>
>>> Hmm, we can either reuse err.
>>>
>>
>> But return value of asprintf() can be more 1,
>> when it succeeded.
>> So I think it is better the below you said.
>
> What I meant was to simply reset err = 0; and set it if we failed and
> returned that. No need to add more variables.
>

I understood ! :)

>>
>>>> int len = strlen(path) + strlen(name) + 2;
>>>> int nlen = strlen(name) + 1;
>>>> char *full = malloc(len);
>>>> @@ -50,12 +50,13 @@ static int load_plugin(struct pevent *pevent, const char *path,
>>>> if (!res) {
>>>> fprintf(stderr, "failed loading %s\n", full);
>>>> PyErr_Print();
>>>> + ret = -1;
>>>> } else
>>>> Py_DECREF(res);
>>>>
>>>> free(load);
>>>>
>>>> - return 0;
>>>> + return ret;
>>>
>>> or do a: return res ? 0 : -1;
>>>
>>> -- Steve
>>>
>>
>> Okey! I'll resent the patch after modifying this!
>
> But this is fine as well.
>
> -- Steve
>

Thank you !!
Have a nice day :)

Taeung