Fixed: potential null reference in XmlTreeGen when DataSet not defined

[sstronin]

There are some null checks probably missing for internal DataSet reference

[ghost]

Tagging subscribers to this area: @roji, @ajcvickers
See info in area-owners.md if you want to be subscribed.

Issue Details

---

There are some null checks probably missing for internal DataSet reference

Author:	sstronin
Assignees:	-
Labels:	area-System.Data
Milestone:	-

[roji]

@sstronin are you able to reproduce the bug in a test? If not, this is generally old code that we're no longer actively developing, and we'd like prefer to not change it (and risk breakage) for a theoretical problem that doesn't exist in practice...

[sstronin]

This was found by a code analyzer and it looks obvious enough. If a test is required, I'll elaborate the issue a bit later

[roji]

This was found by a code analyzer and it looks obvious enough.

                else if(_ds != null)

This change doesn't look like a fully obvious, automated change - it's a behavioral change that results in a different namespace being returned. I'm not saying the change necessarily isn't correct, but I'd want us to fully understand the impact on user behavior and the benefit of the change. As I wrote above, this is mostly considered legacy code by now, and avoiding accidental regressions and behavioral changes is prioritized over changes where there's no clear benefit.

[stephentoub]

This change doesn't look like a fully obvious, automated change - it's a behavioral change that results in a different namespace being returned. I'm not saying the change necessarily isn't correct, but I'd want us to fully understand the impact on user behavior and the benefit of the change. As I wrote above, this is mostly considered legacy code by now, and avoiding accidental regressions and behavioral changes is prioritized over changes where there's no clear benefit.

Given this changes behavior, we don't fully understand the impact, there's no existing test coverage plus no new tests being added, and it hasn't progressed in several months, I'm going to close this. We can reopen/revisit when it's appropriate to do so. @sstronin, thanks for trying to improve things here.