Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.
/ corefx Public archive

PVS-Studio: fixed vulnerability CWE-476 (NULL Pointer Dereference) #16807

Merged
merged 10 commits into from Mar 10, 2017
Merged

PVS-Studio: fixed vulnerability CWE-476 (NULL Pointer Dereference) #16807

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
715dae2
PVS-Studio: fixed vulnerability CWE-476 (NULL Pointer Dereference)
Mar 7, 2017
0e6da0e
PVS-Studio: fixed vulnerability CWE-476 (NULL Pointer Dereference) (P…
Mar 7, 2017
81177fa
Undo changes for understand what is breaking tests.
Mar 9, 2017
7de229d
Fix possible NULL reference exception in SimpleType.cs
Mar 9, 2017
785d16f
Fix possible NULL reference exception in MemberLookup.cs
Mar 9, 2017
0a3b466
Fixed typo in PropertyTabAttribute.cs
Mar 9, 2017
19af27a
Fixed typo in PropertyTabAttribute.cs
Mar 9, 2017
641cbb8
Fix possible NullReferenceException in CompilerInfo.cs and CaseInsens…
Mar 9, 2017
4242d28
Merge branch 'master' of https://github.com/viva64/corefx
Mar 9, 2017
8864066
Fix possible NullReferenceException in NamespaceSortOrder.cs and Attr…
Mar 9, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Common/src/System/Net/CaseInsensitiveAscii.cs
View file
Open in desktop
Expand Up @@ -43,7 +43,7 @@ internal class CaseInsensitiveAscii : IEqualityComparer, IComparer
public int GetHashCode(object myObject)
{
string myString = myObject as string;
if (myObject == null)
if (myString == null)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is better to use object.ReferenceEquals to avoid calling the equality operator

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Roslyn is smart enough to avoid the string equality operator when comparing null, so object.ReferenceEquals isn't necessary.

{
return 0;
}
Expand Down
2 changes: 1 addition & 1 deletion src/System.CodeDom/src/System/CodeDom/Compiler/CompilerInfo.cs
View file
Open in desktop
Expand Up @@ -103,7 +103,7 @@ internal CompilerInfo(CompilerParameters compilerParams, string codeDomProviderT
public override bool Equals(object o)
{
CompilerInfo other = o as CompilerInfo;
if (o == null)
if (other == null)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fact that all of our tests were passing before this change suggests that we're missing some test coverage. To go along with your fixes, can you add tests that would have failed before the changes?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would be glad to do this, but I doubt that my manager would approve these actions. Most likely he will say something "We are a small company and cannot develop Microsoft projects for free" :) If you wish, I could give you contacts of our CEO and you could discuss variants of paid collaboration. We really could fix all the warnings and write tests, we do such tasks. Here is the example of how we did it for Unreal Engine.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general, we consider src/tests to be a unit, such that we ask contributors changing src to ensure that the tests for the thing being changed are appropriate. Doing so helps to ensure that the fix is actually a fix (and not a regression, as it was in the case of one of your changes you backed out... imagine how bad things could have been if we didn't have those tests and we merged the change). We can skip doing so this time for the few remaining modifications, but for any future contributions please do so. Thanks for the interest in contributing!

{
return false;
}
Expand Down
2 changes: 1 addition & 1 deletion ...tem.ComponentModel.TypeConverter/src/System/ComponentModel/Design/PropertyTabAttribute.cs
View file
Open in desktop
Expand Up @@ -222,7 +222,7 @@ private void InitializeArrays(string[] tabClassNames, Type[] tabClasses, Propert
}
else if (tabClassNames != null)
{
if (tabScopes != null && tabClasses.Length != tabScopes.Length)
if (tabScopes != null && tabClassNames.Length != tabScopes.Length)
{
throw new ArgumentException(SR.PropertyTabAttributeArrayLengthMismatch);
}
Expand Down
2 changes: 1 addition & 1 deletion src/System.Data.Common/src/System/Data/SimpleType.cs
View file
Open in desktop
Expand Up @@ -364,7 +364,7 @@ internal string HasConflictingDefinition(SimpleType otherSimpleType)

if (!string.Equals(BaseType, otherSimpleType.BaseType, StringComparison.Ordinal))
return ("BaseType");
if ((BaseSimpleType == null && otherSimpleType.BaseSimpleType != null) &&
if ((BaseSimpleType != null && otherSimpleType.BaseSimpleType != null) &&
(BaseSimpleType.HasConflictingDefinition(otherSimpleType.BaseSimpleType)).Length != 0)
return ("BaseSimpleType");
return string.Empty;
Expand Down
2 changes: 1 addition & 1 deletion ...stem.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/AttributeSortOrder.cs
View file
Open in desktop
Expand Up @@ -19,7 +19,7 @@ public int Compare(object a, object b)
{
XmlNode nodeA = a as XmlNode;
XmlNode nodeB = b as XmlNode;
if ((a == null) || (b == null))
if ((nodeA == null) || (nodeB == null))
throw new ArgumentException();
int namespaceCompare = string.CompareOrdinal(nodeA.NamespaceURI, nodeB.NamespaceURI);
if (namespaceCompare != 0) return namespaceCompare;
Expand Down
2 changes: 1 addition & 1 deletion ...stem.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/NamespaceSortOrder.cs
View file
Open in desktop
Expand Up @@ -18,7 +18,7 @@ public int Compare(object a, object b)
{
XmlNode nodeA = a as XmlNode;
XmlNode nodeB = b as XmlNode;
if ((a == null) || (b == null))
if ((nodeA == null) || (nodeB == null))
throw new ArgumentException();
bool nodeAdefault = Utils.IsDefaultNamespaceNode(nodeA);
bool nodeBdefault = Utils.IsDefaultNamespaceNode(nodeB);
Expand Down