[Mono-dev] Submitting patch?

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.

--rob

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
>     reply]
>
>
>     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.
>
>     -Rob
>
>     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.
>>
>>         -Rob
>>
>>         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...
>>>
>>>         Mono.Data.TdsClient.TdsConnectionPool.cs
>>>
>>>         In the above file, in GetConnection(), either before:
>>>
>>>         goto retry
>>>
>>>         or after the initial
>>>
>>>         retry:
>>>
>>>         (either place should be fine)
>>>
>>>         result needs to be set to null -- that is:
>>>
>>>         result=null;
>>>
>>>         (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.
>>>
>>>         -Rob
>>
>>
>>         _______________________________________________
>>         Mono-devel-list mailing list
>>         Mono-devel-list at lists.ximian.com
>>         <mailto:Mono-devel-list at lists.ximian.com>
>>         http://lists.ximian.com/mailman/listinfo/mono-devel-list
>>
>>
>
>
>     _______________________________________________
>     Mono-devel-list mailing list
>     Mono-devel-list at lists.ximian.com
>     <mailto:Mono-devel-list at lists.ximian.com>
>     http://lists.ximian.com/mailman/listinfo/mono-devel-list
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ximian.com/pipermail/mono-devel-list/attachments/20120510/9cd795c7/attachment-0001.html>