[Mono-dev] Submitting patch?
robwilkens at gmail.com
Fri May 11 00:38:00 UTC 2012
Well, i added the test about a week ago. The test works for me, it
demonstrates failure (a null reference exception) in the case where the
patch i wrote isn't applied, and it passes when the patch is applied. I
realize i'm the only one to report this problem, so it's low priority,
but i wanted to mention to whomever is responsible that if they have any
questions, i'm here for now (that is: I still subscribe to the list).
To me, again, the patch is straightforward and obviously needed --
There's a retry point and the first thing it checks in retry is if
'result==null' (to enter the while loop) and that retry point is only
hit under one circumstance, and in that circumstance, result is never
null, so as a result, if the intention was for it to be anything other
than null the retry point would be elsewhere. Therefore, it should be
blatantly obvious to anyone reading the code, that setting result to
null so the check at the retry point can enter the while loop at the
retry point, is the only thing that is needed to make it work.
To debug this originally, i inserted trace points to confirm what was
going on. It was stuck endlessly jumping to the retry point and not
doing anything other than jumping directly to the retry point again and
again until it got tired of that and failed, and every time after the
first, it never enterred the while loop at the retry point, begging the
question, if null wasn't intended (which it was), why not move the retry
point elsewhere below the while loop.
If someone thinks this breaks something else, please let me know.
It's on github - https://github.com/mono/mono/pull/283 ... I understand
if my nunit test is the reason it is rejected, it was the most
quick/straightforward way to do the tests, it was extracted from
sqlconnection.open() extracting only the code that was required to set
up the TdsConnectionPool GetConnection() call it uses.
On 05/02/2012 07:28 AM, Rodrigo Kumpera wrote:
> Patches are much more likely to be accepted if they include tests.
> It's up to the TDS driver maintainers.
> On Wed, May 2, 2012 at 8:26 AM, Rob Wilkens <robwilkens at gmail.com
> <mailto:robwilkens at gmail.com>> wrote:
> [resending reply via reply to list only, left off list in previous
> I already did the pull request on github -- and i'm not sure how
> to include the tests (I only commented that i ran the tests on
> github in the pull comments). Or do you mean i have to somehow
> generate my own tests, which would be difficult because this
> requires, i think, a functional microsoft sql server for it to
> have access to to reproduce the error.
> On 05/02/2012 07:21 AM, Rodrigo Kumpera wrote:
>> Please make it a pull request on github with tests included.
>> On Tue, May 1, 2012 at 12:55 PM, Rob Wilkens
>> <robwilkens at gmail.com <mailto:robwilkens at gmail.com>> wrote:
>> Ok, I looked up the 'selfish' way to submit a patch from the
>> contributing section, and i think the below suffices as a
>> patch, does anyone disagree?
>> I am referring to : http://www.mono-project.com/Contributing
>> towards the bottom of the page where it simply says to submit
>> the patch to the mailing list.
>> Does the below suffice as a "patch" or should i figure out
>> the 'github' way which i thought i saw elsewhere.
>> On 05/01/2012 11:49 AM, Rob Wilkens wrote:
>>> I found out the fix for the error i reported with multiple invalid
>>> login attempts... It's very simple...
>>> In the above file, in GetConnection(), either before:
>>> goto retry
>>> or after the initial
>>> (either place should be fine)
>>> result needs to be set to null -- that is:
>>> (in my testing, i put it before goto retry)
>>> Otherwise, it keeps retrying because result has never been reset
>>> before the while loop you are trying to enter.
>>> I guess my next step is figuring out how to navigate github so i can submit it.
>>> Sorry for being so public and crowding the mailing list with this. If
>>> someone else wants to volunteer to submit the patch for me, please
>>> speak up. I'll otherwise figure it out.
>> Mono-devel-list mailing list
>> Mono-devel-list at lists.ximian.com
>> <mailto:Mono-devel-list at lists.ximian.com>
> Mono-devel-list mailing list
> Mono-devel-list at lists.ximian.com
> <mailto:Mono-devel-list at lists.ximian.com>
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the Mono-devel-list