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

Fix uninitialised ptr compiler warning #190

Merged
merged 1 commit into from
Feb 6, 2024
Merged

Conversation

jfowkes
Copy link
Contributor

@jfowkes jfowkes commented Feb 6, 2024

As reported by @nimgould compilers currently raise a warning for this section of BuddyAllocator.hxx:

   void* allocate(std::size_t sz) {
      // Try allocating in existing pages
      spral::omp::AcquiredLock scopeLock(lock_);
      void* ptr;
      for(auto& page: pages_) {
         ptr = page.allocate(sz);
         if(ptr) break; // allocation suceeded
      }
      if(!ptr) {
         // Failed to alloc on existing page: make a bigger page and use it

that the pointer may be used uninitialised:

warning: ‘ptr’ may be used uninitialized in this function
|       if(!ptr) {

This PR explicitly initialises the pointer to nullptr rather than expecting the compiler to do this.

@jfowkes jfowkes requested a review from mjacobse February 6, 2024 08:18
@jfowkes jfowkes self-assigned this Feb 6, 2024
Copy link
Collaborator

@mjacobse mjacobse left a comment

Choose a reason for hiding this comment

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

Don't think this warning reveals an actual issue, since the constructor of Table guarantees that pages_ has at least one element. But I agree that explicit initialization makes this clearer not only to the compiler, but also to a human reader.

@jfowkes
Copy link
Contributor Author

jfowkes commented Feb 6, 2024

Agreed thanks @mjacobse!

@jfowkes jfowkes merged commit e45cc03 into master Feb 6, 2024
16 checks passed
@jfowkes jfowkes deleted the fix-compiler-warning branch February 6, 2024 09:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants