Pour obtenir une clé
d'essai remplissez le formulaire ci-dessous
Demandez des tariffs
Nouvelle licence
Renouvellement de licence
--Sélectionnez la devise--
USD
EUR
RUB
* En cliquant sur ce bouton, vous acceptez notre politique de confidentialité

Free PVS-Studio license for Microsoft MVP specialists
To get the licence for your open-source project, please fill out this form
** En cliquant sur ce bouton, vous acceptez notre politique de confidentialité.

I am interested to try it on the platforms:
** En cliquant sur ce bouton, vous acceptez notre politique de confidentialité.

Votre message a été envoyé.

Nous vous répondrons à


Si vous n'avez toujours pas reçu de réponse, vérifiez votre dossier
Spam/Junk et cliquez sur le bouton "Not Spam".
De cette façon, vous ne manquerez la réponse de notre équipe.

>
>
>
Top 10 C# projects errors found in 2016

Top 10 C# projects errors found in 2016

27 Fév 2017

To measure the efficiency of our analyzer, and also to promote the methodology of static analysis, we regularly analyze open source projects for bugs and write articles about the results. 2016 was no exception. This year is especially important as it is the year of the "growth" of the C# analyzer. PVS-Studio has obtained a large number of new C# diagnostics, an improved virtual values mechanism (symbolic execution) and much more. Based on the results of our teamwork, I compiled a kind of chart of the most interesting bugs, found in various C# projects in 2016.

0480_Top_10_CSharp_Bugs_2016/image1.png

Tenth place: when a minute doesn't always have 60 seconds

I'll start the chart with a bug from the Orchard CMS project. The description of the error can be found in this article. In general, the full list of all the articles we checked is here.

V3118 Seconds component of TimeSpan is used, which does not represent full time interval. Possibly 'TotalSeconds' value was intended instead. AssetUploader.cs 182

void IBackgroundTask.Sweep()
{ 
  ....
  // Don't flood the database with progress updates; 
  // Limit it to every 5 seconds.
  if ((_clock.UtcNow - lastUpdateUtc).Seconds >= 5)
  {
     ....
  }
}

The developer mistakenly used Seconds instead of TotalSeconds in this case. Thus, we will get not the full number of seconds between the dates _clock.UtcNow and lastUpdateUtc, as the developer expected, but only the Seconds component of the interval. For example, for the time interval of 1 minute 4 seconds it will be not 64 seconds, but 4 seconds. Incredible, but even experienced developers make such mistakes.

Ninth place: the expression is always true

The following error is from the article about the analysis of GitExtensions.

V3022 Expression 'string.IsNullOrEmpty(rev1) || string.IsNullOrEmpty(rev2)' is always true. GitUI FormFormatPatch.cs 155

string rev1 = "";
string rev2 = "";

var revisions = RevisionGrid.GetSelectedRevisions();
if (revisions.Count > 0)
{
  rev1 = ....;
  rev2 = ....;
  ....
}
else

if (string.IsNullOrEmpty(rev1) || string.IsNullOrEmpty(rev2)) // <=
{
    MessageBox.Show(....);
    return;
}

Note the else keyword. Most likely, this is not the right place for it. Inattention during refactoring, or just banal tiredness of a programmer, and we are getting a radical change in the logic of the program, which leads to unpredictable behavior. It's great that a static analyzer is never tired.

Eighth place: a possible typo

In the article about the analysis of FlashDevelop source code, we see a peculiar error, caused by a typo.

V3056 Consider reviewing the correctness of 'a1' item's usage. LzmaEncoder.cs 225

public void SetPrices(....)
{
    UInt32 a0 = _choice.GetPrice0();
    UInt32 a1 = _choice.GetPrice1();
    UInt32 b0 = a1 + _choice2.GetPrice0();  // <=
    UInt32 b1 = a1 + _choice2.GetPrice1();
    ....
}

I agree with the analyzer, as well as the author of the article. It feels like a0 should be used instead of the a1 variable in the marked line. In any case, it wouldn't hurt to give more meaningful names to variables.

Seventh place: logic error

This article was written based on the second check of the Umbraco project. An example of an interesting, in my opinion, error from it.

V3022 Expression 'name != "Min" || name != "Max"' is always true. Probably the '&&' operator should be used here. DynamicPublishedContentList.cs 415

private object Aggregate(....)
{
  ....
  if (name != "Min" || name != "Max")
  {
    throw new ArgumentException(
      "Can only use aggregate min or max methods on properties
       which are datetime");
  }
  ....
}

An exception of the ArgumentException type will be thrown for any value of name variable. It's because of the erroneous use of the || operator instead of && in the condition.

Sixth place: incorrect loop condition

The article about the check of Accord.Net has a description of several amusing bugs. I have chosen two, one of which is related to a typo again.

V3015 It is likely that the wrong variable is being compared inside the 'for' operator. Consider reviewing 'i' Accord.Audio SampleConverter.cs 611

public static void Convert(float[][] from, short[][] to)
{
  for (int i = 0; i < from.Length; i++)
    for (int j = 0; i < from[0].Length; j++)
      to[i][j] = (short)(from[i][j] * (32767f));
}

There is an error in the condition of the second for loop, whose counter is the j variable. Using the variable names i and j for the counters is some sort of classic of programming. Unfortunately, these variables are very similar looking, so the developers often make mistakes in such code. I do not think that in this case it is worth any recommendation to use more meaningful names. Nobody will do that anyway. So, here is another recommendation: use static analyzers!

0480_Top_10_CSharp_Bugs_2016/image2.png

Fifth place: using a bitwise operator instead of a logical operator

One more interesting and quite a wide-spread error from the article about the analysis of Accord.Net.

V3093 The '&' operator evaluates both operands. Perhaps a short-circuit '&&' operator should be used instead. Accord.Math JaggedSingularValueDecompositionF.cs 461

public JaggedSingularValueDecompositionF(....)
{
  ....
  if ((k < nct) & (s[k] != 0.0))
  ....
}

It is obvious that even if the first condition is true, the erroneous use of & instead of && will lead to the check of the second condition, which in turn, will cause array index out of bounds.

Fourth place: quotation mark and... a quotation mark again

The fourth place is taken by the Xamarin.Forms project; the full article about its check is here.

V3038 The first argument of 'Replace' function is equal to the second argument. ICSharpCode.Decompiler ReflectionDisassembler.cs 349

void WriteSecurityDeclarationArgument(CustomAttributeNamedArgument na)
{
  ....
  output.Write("string('{0}')",
    NRefactory.CSharp
.TextWriterTokenWriter
.ConvertString(
(string)na.Argument.Value).Replace("'", "\'"));
  ....
}

In this case, the quotation mark will be replaced with... a quotation mark. I do not think that this is what the developer wanted.

0480_Top_10_CSharp_Bugs_2016/image3.png

The analyzer did a great job!

Third place: ThreadStatic

In third place, is the Mono project that was also rich in amusing bugs. Here is the article about its check. One of these bugs is a real rarity.

V3089 Initializer of a field marked by [ThreadStatic] attribute will be called once on the first accessing thread. The field will have default value on different threads. System.Data.Linq-net_4_x Profiler.cs 16

static class Profiler
{
  [ThreadStatic]
  private static Stopwatch timer = new Stopwatch();
  ....
}

In a nutshell: the field marked by the ThreadStatic attribute is initialized incorrectly. In the documentation of the diagnostic you may see a detailed description of the situation, and some tips on how to avoid such errors. It's the perfect example of an error that is not that easy to find and fix using the usual methods.

Second place: Copy-Paste - classic!

One of the classical, to my mind, errors of the Copy-Paste type which was found in the already mentioned Mono project (see the full article).

V3012 The '?:' operator, regardless of its conditional expression, always returns one and the same value: Color.FromArgb (150, 179, 225). ProfessionalColorTable.cs 258

Here is a short code fragment, where this bug was found:

button_pressed_highlight = use_system_colors ?
                           Color.FromArgb (150, 179, 225) : 
                           Color.FromArgb (150, 179, 225);

You may ask: "Is it really such a huge mistake?" "Does this obvious mistake really deserve second place in the chart?" The thing is that this code fragment was purposefully formatted to make it more visible. And now imagine that you have the task of finding an error like this without any tools. So, those with weak nerves, we would ask not to look at this; here is a screenshot of a full code fragment with an error (click on the image to enlarge):

0480_Top_10_CSharp_Bugs_2016/image4.png

Apparently, this mistake can be done by any programmer, regardless of the qualification. Successfully finding such errors demonstrates the full power of static analysis.

First place: PVS-Studio

No, it's not an illusion. "PVS-Studio" really is written here. It took first place in our chart. Not because it is a great static analyzer, but because our team has ordinary people, who make simple human errors when writing code. This was the main topic of an article we wrote previously. With the help of PVS-Studio, we detected two errors in the code of PVS-Studio itself.

V3022 Expression 'RowsCount > 100000' is always false. ProcessingEngine.cs 559

V3022 Expression 'RowsCount > 200000' is always false. ProcessingEngine.cs 561

public void ProcessFiles(....)
{
  ....
  int RowsCount = 
    DynamicErrorListControl.Instance.Plog.NumberOfRows;
  if (RowsCount > 20000)
    DatatableUpdateInterval = 30000; //30s
  else if (RowsCount > 100000)
    DatatableUpdateInterval = 60000; //1min
  else if (RowsCount > 200000)
    DatatableUpdateInterval = 120000; //2min
  ....
}

The result of this code fragment (on condition that RowsCount > 20000) will always be a value DatatableUpdateInterval equal to 30000.

Fortunately, we have already done some work in this sphere.

0480_Top_10_CSharp_Bugs_2016/image6.png

Thanks to widely used incremental analysis in our team, the articles "bugs in PVS-Studio found by PVS-Studio" are very unlikely to appear.

Conclusion

I should note that anyone can make a personal chart of bugs found using a free version of PVS-Studio static analyzer for checking individual projects.

Download and try PVS-Studio: http://www.viva64.com/en/pvs-studio/

To purchase a commercial license, please contact us via email. You can also write to us to get a temporary license key for a comprehensive investigation of PVS-Studio, if you want to avoid the limitations of the demo version.

Popular related articles
Sorting in C#: OrderBy.OrderBy or OrderBy.ThenBy? What's more effective and why?

Date: 20 Sep 2022

Author: Sergey Vasiliev

Suppose we need to sort the collection by multiple keys. In C#, we can do this with the help of OrderBy().OrderBy() or OrderBy().ThenBy(). But what is the difference between these calls? To answer th…
ML.NET: can Microsoft's machine learning be trusted?

Date: 08 Sep 2022

Author: Andrey Moskalev

In 2018, Microsoft created ML.NET, a machine learning framework for .NET developers. Since then, the machine learning library has undergone significant changes and acquired new features to identify p…
The risks of using vulnerable dependencies in your project, and how SCA helps manage them

Date: 06 Sep 2022

Author: Nikita Lipilin

Most applications today use third-party libraries. If such a library contains a vulnerability, an app that uses this library may also be vulnerable. But how can you identify such problematic dependen…
Build to order? Checking MSBuild for the second time

Date: 01 Sep 2022

Author: Nikita Panevin

MSBuild is a popular open-source build platform created by Microsoft. Developers all over the world use MSBuild. In 2016, we checked it for the first time and found several suspicious places. Can we …
The Orchard Core threequel. Rechecking the project with PVS-Studio

Date: 25 Aoû 2022

Author: Aleksey Avdeev

In this article, we check the Orchard Core project with the help of the PVS-Studio static analyzer. We are going to find out if the platform code is as good as the sites created on its basis. May the…

Comments (0)

Next comments
Unicorn with delicious cookie
Nous utilisons des cookies pour améliorer votre expérience de navigation. En savoir plus
Accepter