Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

To do: Review library code and remove any uninitialized memory access #583

Closed
cookpa opened this issue Mar 18, 2024 · 5 comments · Fixed by #590
Closed

To do: Review library code and remove any uninitialized memory access #583

cookpa opened this issue Mar 18, 2024 · 5 comments · Fixed by #590
Assignees

Comments

@cookpa
Copy link
Member

cookpa commented Mar 18, 2024

Relating to #579. Further fixes may be pending on the ANTs side (ANTsX/ANTs#1701)

@cookpa
Copy link
Member Author

cookpa commented Mar 19, 2024

Additional ANTs fixes ready for update here ANTsX/ANTs#1702

@cookpa
Copy link
Member Author

cookpa commented Mar 20, 2024

Going through the C++ code replacing Allocate with AllocateInitialized - just wanted to note this

https://github.com/ANTsX/ANTsPy/blame/1e363730d033500a91264e4ab6509f093dd609ae/ants/lib/LOCAL_antsImageToImageMetric.h#L357-L363

a prior call to FillBuffer is commented out for some reason. Hopefully just a performance thing

@ntustison
Copy link
Member

Im guessing I just missed that one.

@cookpa
Copy link
Member Author

cookpa commented Mar 20, 2024

@ntustison this is in the ANTsPy code, I think you got everything in ANTsX/ANTs

I'm trying to figure out if images with Vector pixel type require special handling. Elsewhere we have

    VectorType zeroVector( 0.0 );
    ITKFieldPointerType rasterizedField = ITKFieldType::New();
    rasterizedField->SetOrigin( bsplineFilter->GetBSplineDomainOrigin() );
    rasterizedField->SetSpacing( bsplineFilter->GetBSplineDomainSpacing() );
    rasterizedField->SetDirection( bsplineFilter->GetBSplineDomainDirection() );
    rasterizedField->SetRegions( bsplineFilter->GetBSplineDomainSize() );
    rasterizedField->Allocate();
    rasterizedField->FillBuffer( zeroVector );

I am hoping we can just call rasterizedField->AllocateInitialized. I think this is the case because itkImage must be aware of the total buffer size it needs to initialize

@ntustison
Copy link
Member

Okay, was on my phone. There's quite a few code blocks like this where we're switching back and forth from an image of vectors to a vector image. I would assume there's nothing complicated going on and whatever changes you want to make should be fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants