Possible NPE in converters

[Felix Schumacher <fe...@internetallee.de>]

Hi all,

in the converters in org.apache.jmeter.save.converters there are a few 
possible NPE. In the method canConvert(Class) often the implementation 
looks like

  "return arg0.equals(SomeClass.class)"

without arg0 being checked for non-null.

I have a patch, that reverts the logic to

  "return SomeClass.class.equals(arg0)"

This is null safe and should return the same values (apart from possible 
NPE).

Can I check those changes in whithout creating a bugzilla entry?

Regards
  Felix

Re: Possible NPE in converters

[Philippe Mouawad <ph...@gmail.com>]

You never know :)
Sometimes a slight corr change can break things.
As this is really core and critical part, it doesn't hurt to mention it.

Regards

On Monday, December 22, 2014, sebb <se...@gmail.com> wrote:

> On 22 December 2014 at 18:31, Philippe Mouawad
> <philippe.mouawad@gmail.com <javascript:;>> wrote:
> > If you add a Bugzilla, it will be in changes.html so users see it.
> > while here only subscriber to commits will see it.
>
> Yes, but is this particular change likely to be relevant to end users?
> I'm not sure the NPE can occur in normal use.
>
> >
> >
> > On Monday, December 22, 2014, sebb <sebbaz@gmail.com <javascript:;>>
> wrote:
> >
> >> On 22 December 2014 at 17:05, Philippe Mouawad
> >> <philippe.mouawad@gmail.com <javascript:;> <javascript:;>> wrote:
> >> > Hi,
> >> > I prefer if possible to open a Bugzilla, as this kind of change can be
> >> > impacting and sometimes you notice issues few months after but don't
> >> > remember this change.
> >>
> >> I don't see how a Bugzilla helps here, so long as the log message is
> clear.
> >>
> >> I would not expect to raise a Bugzilla for trivial spelling errors,
> >> nor tab removal.
> >> Nor is it always necessary for simple changes such as converting magic
> >> numbers or strings to private constants.
> >>
> >> > This is particularly needed for Third Party providers.
> >>
> >> What do you mean by that?
> >>
> >> > Regards
> >> > Philippe
> >> >
> >> > On Mon, Dec 22, 2014 at 5:57 PM, sebb <sebbaz@gmail.com
> <javascript:;> <javascript:;>>
> >> wrote:
> >> >
> >> >> On 22 December 2014 at 16:34, Felix Schumacher
> >> >> <felix.schumacher@internetallee.de <javascript:;> <javascript:;>>
> wrote:
> >> >> > Hi all,
> >> >> >
> >> >> > in the converters in org.apache.jmeter.save.converters there are a
> few
> >> >> > possible NPE. In the method canConvert(Class) often the
> implementation
> >> >> looks
> >> >> > like
> >> >> >
> >> >> >  "return arg0.equals(SomeClass.class)"
> >> >> >
> >> >> > without arg0 being checked for non-null.
> >> >> >
> >> >> > I have a patch, that reverts the logic to
> >> >> >
> >> >> >  "return SomeClass.class.equals(arg0)"
> >> >> >
> >> >> > This is null safe and should return the same values (apart from
> >> possible
> >> >> > NPE).
> >> >> >
> >> >> > Can I check those changes in whithout creating a bugzilla entry?
> >> >>
> >> >> OK by me.
> >> >>
> >> >> Please ensure that the commit only contains these specific changes,
> >> >> and not any unrelated ones.
> >> >> Thanks.
> >> >>
> >> >> > Regards
> >> >> >  Felix
> >> >>
> >> >
> >> >
> >> >
> >> > --
> >> > Cordialement.
> >> > Philippe Mouawad.
> >>
> >
> >
> > --
> > Cordialement.
> > Philippe Mouawad.
>


-- 
Cordialement.
Philippe Mouawad.

Re: Possible NPE in converters

[sebb <se...@gmail.com>]

On 22 December 2014 at 18:31, Philippe Mouawad
<ph...@gmail.com> wrote:
> If you add a Bugzilla, it will be in changes.html so users see it.
> while here only subscriber to commits will see it.

Yes, but is this particular change likely to be relevant to end users?
I'm not sure the NPE can occur in normal use.

>
>
> On Monday, December 22, 2014, sebb <se...@gmail.com> wrote:
>
>> On 22 December 2014 at 17:05, Philippe Mouawad
>> <philippe.mouawad@gmail.com <javascript:;>> wrote:
>> > Hi,
>> > I prefer if possible to open a Bugzilla, as this kind of change can be
>> > impacting and sometimes you notice issues few months after but don't
>> > remember this change.
>>
>> I don't see how a Bugzilla helps here, so long as the log message is clear.
>>
>> I would not expect to raise a Bugzilla for trivial spelling errors,
>> nor tab removal.
>> Nor is it always necessary for simple changes such as converting magic
>> numbers or strings to private constants.
>>
>> > This is particularly needed for Third Party providers.
>>
>> What do you mean by that?
>>
>> > Regards
>> > Philippe
>> >
>> > On Mon, Dec 22, 2014 at 5:57 PM, sebb <sebbaz@gmail.com <javascript:;>>
>> wrote:
>> >
>> >> On 22 December 2014 at 16:34, Felix Schumacher
>> >> <felix.schumacher@internetallee.de <javascript:;>> wrote:
>> >> > Hi all,
>> >> >
>> >> > in the converters in org.apache.jmeter.save.converters there are a few
>> >> > possible NPE. In the method canConvert(Class) often the implementation
>> >> looks
>> >> > like
>> >> >
>> >> >  "return arg0.equals(SomeClass.class)"
>> >> >
>> >> > without arg0 being checked for non-null.
>> >> >
>> >> > I have a patch, that reverts the logic to
>> >> >
>> >> >  "return SomeClass.class.equals(arg0)"
>> >> >
>> >> > This is null safe and should return the same values (apart from
>> possible
>> >> > NPE).
>> >> >
>> >> > Can I check those changes in whithout creating a bugzilla entry?
>> >>
>> >> OK by me.
>> >>
>> >> Please ensure that the commit only contains these specific changes,
>> >> and not any unrelated ones.
>> >> Thanks.
>> >>
>> >> > Regards
>> >> >  Felix
>> >>
>> >
>> >
>> >
>> > --
>> > Cordialement.
>> > Philippe Mouawad.
>>
>
>
> --
> Cordialement.
> Philippe Mouawad.

Re: Possible NPE in converters

[Philippe Mouawad <ph...@gmail.com>]

If you add a Bugzilla, it will be in changes.html so users see it.
while here only subscriber to commits will see it.



On Monday, December 22, 2014, sebb <se...@gmail.com> wrote:

> On 22 December 2014 at 17:05, Philippe Mouawad
> <philippe.mouawad@gmail.com <javascript:;>> wrote:
> > Hi,
> > I prefer if possible to open a Bugzilla, as this kind of change can be
> > impacting and sometimes you notice issues few months after but don't
> > remember this change.
>
> I don't see how a Bugzilla helps here, so long as the log message is clear.
>
> I would not expect to raise a Bugzilla for trivial spelling errors,
> nor tab removal.
> Nor is it always necessary for simple changes such as converting magic
> numbers or strings to private constants.
>
> > This is particularly needed for Third Party providers.
>
> What do you mean by that?
>
> > Regards
> > Philippe
> >
> > On Mon, Dec 22, 2014 at 5:57 PM, sebb <sebbaz@gmail.com <javascript:;>>
> wrote:
> >
> >> On 22 December 2014 at 16:34, Felix Schumacher
> >> <felix.schumacher@internetallee.de <javascript:;>> wrote:
> >> > Hi all,
> >> >
> >> > in the converters in org.apache.jmeter.save.converters there are a few
> >> > possible NPE. In the method canConvert(Class) often the implementation
> >> looks
> >> > like
> >> >
> >> >  "return arg0.equals(SomeClass.class)"
> >> >
> >> > without arg0 being checked for non-null.
> >> >
> >> > I have a patch, that reverts the logic to
> >> >
> >> >  "return SomeClass.class.equals(arg0)"
> >> >
> >> > This is null safe and should return the same values (apart from
> possible
> >> > NPE).
> >> >
> >> > Can I check those changes in whithout creating a bugzilla entry?
> >>
> >> OK by me.
> >>
> >> Please ensure that the commit only contains these specific changes,
> >> and not any unrelated ones.
> >> Thanks.
> >>
> >> > Regards
> >> >  Felix
> >>
> >
> >
> >
> > --
> > Cordialement.
> > Philippe Mouawad.
>


-- 
Cordialement.
Philippe Mouawad.

Re: Possible NPE in converters

[sebb <se...@gmail.com>]

On 22 December 2014 at 17:05, Philippe Mouawad
<ph...@gmail.com> wrote:
> Hi,
> I prefer if possible to open a Bugzilla, as this kind of change can be
> impacting and sometimes you notice issues few months after but don't
> remember this change.

I don't see how a Bugzilla helps here, so long as the log message is clear.

I would not expect to raise a Bugzilla for trivial spelling errors,
nor tab removal.
Nor is it always necessary for simple changes such as converting magic
numbers or strings to private constants.

> This is particularly needed for Third Party providers.

What do you mean by that?

> Regards
> Philippe
>
> On Mon, Dec 22, 2014 at 5:57 PM, sebb <se...@gmail.com> wrote:
>
>> On 22 December 2014 at 16:34, Felix Schumacher
>> <fe...@internetallee.de> wrote:
>> > Hi all,
>> >
>> > in the converters in org.apache.jmeter.save.converters there are a few
>> > possible NPE. In the method canConvert(Class) often the implementation
>> looks
>> > like
>> >
>> >  "return arg0.equals(SomeClass.class)"
>> >
>> > without arg0 being checked for non-null.
>> >
>> > I have a patch, that reverts the logic to
>> >
>> >  "return SomeClass.class.equals(arg0)"
>> >
>> > This is null safe and should return the same values (apart from possible
>> > NPE).
>> >
>> > Can I check those changes in whithout creating a bugzilla entry?
>>
>> OK by me.
>>
>> Please ensure that the commit only contains these specific changes,
>> and not any unrelated ones.
>> Thanks.
>>
>> > Regards
>> >  Felix
>>
>
>
>
> --
> Cordialement.
> Philippe Mouawad.

Re: Possible NPE in converters

[Philippe Mouawad <ph...@gmail.com>]

Hi,
I prefer if possible to open a Bugzilla, as this kind of change can be
impacting and sometimes you notice issues few months after but don't
remember this change.
This is particularly needed for Third Party providers.

Regards
Philippe

On Mon, Dec 22, 2014 at 5:57 PM, sebb <se...@gmail.com> wrote:

> On 22 December 2014 at 16:34, Felix Schumacher
> <fe...@internetallee.de> wrote:
> > Hi all,
> >
> > in the converters in org.apache.jmeter.save.converters there are a few
> > possible NPE. In the method canConvert(Class) often the implementation
> looks
> > like
> >
> >  "return arg0.equals(SomeClass.class)"
> >
> > without arg0 being checked for non-null.
> >
> > I have a patch, that reverts the logic to
> >
> >  "return SomeClass.class.equals(arg0)"
> >
> > This is null safe and should return the same values (apart from possible
> > NPE).
> >
> > Can I check those changes in whithout creating a bugzilla entry?
>
> OK by me.
>
> Please ensure that the commit only contains these specific changes,
> and not any unrelated ones.
> Thanks.
>
> > Regards
> >  Felix
>



-- 
Cordialement.
Philippe Mouawad.

Re: Possible NPE in converters

[sebb <se...@gmail.com>]

On 22 December 2014 at 16:34, Felix Schumacher
<fe...@internetallee.de> wrote:
> Hi all,
>
> in the converters in org.apache.jmeter.save.converters there are a few
> possible NPE. In the method canConvert(Class) often the implementation looks
> like
>
>  "return arg0.equals(SomeClass.class)"
>
> without arg0 being checked for non-null.
>
> I have a patch, that reverts the logic to
>
>  "return SomeClass.class.equals(arg0)"
>
> This is null safe and should return the same values (apart from possible
> NPE).
>
> Can I check those changes in whithout creating a bugzilla entry?

OK by me.

Please ensure that the commit only contains these specific changes,
and not any unrelated ones.
Thanks.

> Regards
>  Felix