Menu

Remove unnecessary null pointer checks

2019-12-07
2019-12-15
  • Charles Karney

    Charles Karney - 2019-12-08

    OK, thanks. I've forward this on to Scott Heiman who maintains the .NET version of the library. I assume it's OK to get rid of the NULL check. But I'd like to give Scott a chance to comment.

     
  • Anonymous

    Anonymous - 2019-12-14

    I remember reading (somewhere) that finalizers may be called more than once. Thus, I think the nullptr check is necessary.

    Having said that, I have never seen a finalizer called more than once, so Markus may be correct. If someone can justify the removal of the nullptr check then I will remove them;' otherwise, I see no harm in keeping the check.

     
    • Markus Elfring

      Markus Elfring - 2019-12-14
      • Did you get informed in the way that the delete operation from the language “C++/CLI” would not tolerate the passing of null pointers?
      • Do you find information from the section “15.4.5 Delete” of the standard specification “ECMA-372” sufficient?
       
  • Anonymous

    Anonymous - 2019-12-15

    Section 15.4.5 refers to pointers to managed objects which are allocated on the CLI heap. The pointers that are tested in the finalizer functions point to unmanaged/native objects which are allocated on the native heap. I'm not convinced that 15.4.5 applies to unmanaged objects.

    I check the unmanaged pointers because the samples that I have found on the Internet check the pointers as well.
    https://blogs.msdn.microsoft.com/branbray/2005/07/20/some-notes-about-mixed-types/ search for "!Embedded".

    https://communities.bentley.com/other/old_site_member_blogs/bentley_employees/b/mdanes_blog/posts/idisposable

    https://www.wihlidal.com/files/getd_bonus_chp_2.pdf

     
    • Markus Elfring

      Markus Elfring - 2019-12-15

      Which software behaviour may you expect for the deletion of objects allocated on the CLI heap according to the standard specification?

       
  • Anonymous

    Anonymous - 2019-12-15

    I don't know. As I stated before, the pointers that I am testing and deleting are allocated on the unmanaged heap, not the CLI heap. A better place to ask your question is on the Microsoft .NET forums.

    The first link in my previous post was published by a Microsft employee who helped design .NET. He discusses the rational for implementing Finalizers and why they should be used to free unmanaged memory in managed classes. His discussion may answer your question.

    I believe that I have presented enough information to justify the implementation. The only reason that I will change the implementation is
    1) It is a bug that causes applications to crash or behave in an unexpected manner, or
    2) It seriously degrades application performance.

     
    • Markus Elfring

      Markus Elfring - 2019-12-15

      I don't know.

      I find this information surprising.

      The first link in my previous post was published by a Microsft employee who helped design .NET.

      • I find the usage of deterministic object destruction clear.
      • I wonder also why the author “branbray” suggested in the article “Some Notes about Mixed Types” (from 2005-07-20) to use an extra null pointer check before the C++ delete operator call in the finalizer of the class template “Embedded”.

      2) It seriously degrades application performance.

      • How do you think about to compare run time characteristics for less questionable code?
      • Would you like to reuse smart pointers?
       
  • Anonymous

    Anonymous - 2019-12-15

    Ok, I reread your first post and saw the link on the first line (which I did not see before). Yes, apparently the C++ standard allows you to delete null pointers. I was not aware of this.

    I will remove this unneccessary check in a future release.

     
  • Charles Karney

    Charles Karney - 2019-12-15

    I've updated the dotnet branch for any changes on the .NET library. Also note that I'll require C++11 support in the next version of GeographicLib which means that VS 2013 will be the minimum supported version of Visual Studio.

     

Anonymous
Anonymous

Add attachments
Cancel