Carpet may call object methods with this == NULL

Comments (13)

1. 

   Roland Haas reporter

   • changed status to open
   • removed comment

   • 2015-10-17T13:09:59+00:00
2. 

   Roland Haas reporter

   • changed status to resolved
   • removed comment

   This is likely not worthwhile to address (if indeed it is a bug) unless we encounter a system where this breaks (with a SEGFAULT so it won't be a silent failure). See https://bitbucket.org/eschnett/carpet/pull-requests/7/carpet-avoid-using-null-pointer-for-member/diff#comment-10904860

   • 2016-07-02T09:48:50+00:00
3. 

   Roland Haas reporter

   • changed status to open
   • removed comment

   Fails in CarpetPeriodic test testperiodicinterp. Related to #1821 which is however only a warning.

   • 2017-03-02T16:51:15+00:00
4. 

   Erik Schnetter

   • removed comment

   I now recall the correction I put into place. The fix is applied at the calling side (obviously). Instead of creating a temporary object, we're using another object that already exists at the caller, e.g. src instead of dst.

   I thought this fix was already committed. Maybe I only applied it to a branch, or maybe I missed a calling site?

   • 2017-03-03T14:05:56+00:00
5. 

   Roland Haas reporter

   • removed comment

   maybe, here's another way about it: https://bitbucket.org/eschnett/carpet/pull-requests/7/carpet-avoid-using-null-pointer-for-member/diff

   • 2017-03-03T14:16:23+00:00
6. 

   Erik Schnetter

   • removed comment

   (1) I don't think that CarpetLib itself calls this method, so the changes to ggf.cc should not be necessary. (2) In PeriodicCarpet, you could use src instead of fake_data_pointer(). (3) The initialization of the static variable in fake_data_pointer is not thread-safe. This has actually become an issue when using Qthreads for parallelization.

   • 2017-03-03T16:01:40+00:00
7. 

   Roland Haas reporter

   • removed comment

   (1) I don't think that CarpetLib itself calls this method, so the changes to ggf.cc should not be necessary.

   I added a comment to the code diff: https://bitbucket.org/eschnett/carpet/pull-requests/7/carpet-avoid-using-null-pointer-for-member/diff#comment-33242870

   (2) In PeriodicCarpet, you could use src instead of fake_data_pointer(). Could be done. That seems odd though.

   (3) The initialization of the static variable in fake_data_pointer is not thread-safe. This has actually become an issue when using Qthreads for parallelization. Correct. At the time I reported this CarpetLib was not yet thread safe (ie none of the Timers was) so it did not matter. Thread safe would be either using new() or the src trick that you suggest or making transfer_from a static member function that takes the current dst as eg its first argument.

   • 2017-03-15T15:37:08+00:00
8. 

   Roland Haas reporter

   • changed status to open
   • removed comment

   I updated the pull request with a version that introduces static functions for gdata::copy_from and gdata::transfer_from and renames them to copy_data and transfer_data. This requires changes in all code using those functions since the originals are gone (could be restored but the current state brings attention to possible code parts that may also pass in a NULL this pointer).

   • 2017-05-11T08:26:09+00:00
9. 

   Roland Haas reporter

   • removed comment

   Erik: I think this should be reviewed before the next release as otherwise CarpetPeriodic fails to run when compiled with newer compilers (gcc 6.0 at least).

   • 2017-06-09T09:08:07+00:00
10. 

    Roland Haas reporter

    • changed milestone to ET_2017_05
    • removed comment

    • 2017-06-09T09:33:50+00:00
11. 

    Erik Schnetter

    • changed status to open
    • removed comment

    • 2017-06-12T09:55:10+00:00
12. 

    Roland Haas reporter

    • changed status to resolved
    • removed comment

    Thank you. Applied as git hash 474a09de0c1cd56c00492400fe5304c8e9591cbd "PeriodicCarpet: avoid using NULL this pointer for gdata::copy_from" of Carpet.

    • 2017-06-13T13:51:25+00:00
13. 

    Roland Haas reporter

    • edited description
    • changed status to closed

    • 2019-02-21T20:08:15+00:00
14. Log in to comment