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

[FR] improve bad diagnostic for __REMOVED_IN on ALooper_pollAll #2014

Open
DanAlbert opened this issue Apr 16, 2024 · 8 comments
Open

[FR] improve bad diagnostic for __REMOVED_IN on ALooper_pollAll #2014

DanAlbert opened this issue Apr 16, 2024 · 8 comments

Comments

@DanAlbert
Copy link
Member

DanAlbert commented Apr 16, 2024

Description

error: 'ALooper_pollAll' is unavailable: obsoleted in Android 1
     45 |     if (ALooper_pollAll(IsVulkanReady() ? 1 : 0, nullptr,
        |         ^
  usr/include/android/looper.h:216:5: note: 'ALooper_pollAll' has been explicitly marked unavailable here
    216 | int ALooper_pollAll(int timeoutMillis, int* outFd, int* outEvents, void** outData) __REMOVED_IN(1);
        |     ^

It's been marked obsolete because the implementation is racy and can't be called safely, but ALooper_pollOnce can be called in a loop for the same effect, and is available for all supported OS versions.

The diagnostic is next to useless though, and should be expanded to include that explanation.

I've explained the OS bug in another thread to not bog down this bug (which is about an unclear diagnostic, not a missing API): #2020.

@enh-google
Copy link
Collaborator

is plain old __attribute__((__deprecated__(("use foo instead")))) a better choice then, since it includes a user-provided diagnostic?

@DanAlbert
Copy link
Member Author

__attribute__((availability)) supports that too. We don't expose that in __REMOVED_IN() though, and that's the fix we need.

@enh-google
Copy link
Collaborator

oh, yeah, that seems like a bug in __REMOVED_IN() --- i vote we require an explanation...

@DanAlbert
Copy link
Member Author

Don't know why I didn't think of that, or why we didn't do that in the first place :) +1

@FaisalAhmedAlghamdi

This comment was marked as off-topic.

@enh-google

This comment was marked as off-topic.

@enh-google

This comment was marked as off-topic.

@DanAlbert

This comment was marked as off-topic.

hubot pushed a commit to aosp-mirror/platform_bionic that referenced this issue May 2, 2024
I'll land another patch that makes this required once I've fixed up
all the callers.

Bug: android/ndk#2014
Test: treehugger
Change-Id: I62b9fdd3174f37d33f01c27f7f4e9134f6d9df6e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Unconfirmed
Development

No branches or pull requests

3 participants