Hi Seb,
On 10/29/2015 04:09 PM, Sebastian Kuzminsky wrote:
> On 10/28/2015 12:56 PM, John Morris wrote:
>>
>>> Once you're satisfied the bug actually exists, I'd appreciate another
>>> look at the fix.
>>>
>>> [1]: http://git.linuxcnc.org/gitweb?p=linuxcnc.git;a=commit;h=08dce7b3
>>
>> (One more fixup:)
>> [2]: http://git.linuxcnc.org/gitweb?p=linuxcnc.git;a=commit;h=b714b534
>
> Ok, now I see the bug, and your fix makes sense and looks good.
>
> Can you explain the last hunk of the fix commit, it looks to me like a
> no-op. Maybe leftovers from a previous iteration of the fix?
The last hunk changes an additional `emcStatus->task.readLine` to
`emcTaskPlanLine()`, as well as moves a brace; with the reindentation, I
went ahead and reflowed the comments to fit 80 columns. Moving the
brace may have been an artifact from an older iteration; it's logically
a noop in this patch. The next version will also check
`emcTaskPlanLevel()` in the previous `if` conditional, so I'll revert
some of that.
> The bug is present in 2.6 and later, I'd like to apply the fix (and the
> test) there and merge up. I just pushed a cleaned-up version of the
> test, with your modification that makes it expose the problem.
>
> If you're happy with that proposal, please push the test and the fix to 2.6.
>
> Thanks for all your work on this!
Sounds great! Thank you for the review.
John
------------------------------------------------------------------------------
_______________________________________________
Emc-developers mailing list
Emc-developers@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/emc-developers
