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

Clang-Tidy complaint about re_snprintf #1207

Closed
juha-h opened this issue Oct 22, 2024 · 10 comments · Fixed by #1206
Closed

Clang-Tidy complaint about re_snprintf #1207

juha-h opened this issue Oct 22, 2024 · 10 comments · Fixed by #1206

Comments

@juha-h
Copy link
Contributor

juha-h commented Oct 22, 2024

Android Studio Clang-Tidy complains about all re_snprintf calls. Example is below. Is this a bug in Clang-Tidy or a real issue with the macro?

Clang-Tidy: Conditional operator with identical true and false expressions
Declared in: re_fmt. h
Definition:
#define re_snprintf(str, size, fmt, ...)                                      \
	_re_snprintf_s((str), (size), (fmt), RE_VA_ARGS(__VA_ARGS__))
Replacement:
_re_snprintf_s((event_buf), (sizeof event_buf), ("call closed,%s,%s"),
        _Generic((0) ? (prm) : (prm),
                _Bool: sizeof(int),
                char: sizeof(int),
                unsigned char: sizeof(unsigned int),
                short: sizeof(int),
                unsigned short: sizeof(unsigned int),
                int: sizeof(int),
                unsigned int: sizeof(unsigned int),
                long: sizeof(long),
                unsigned long: sizeof(unsigned long),
                long long: sizeof(long long),
                unsigned long long: sizeof(unsigned long long),
                float: sizeof(double),
                double: sizeof(double),
                char const *: sizeof(char const *),
                char *: sizeof(char *),
                void const *: sizeof(void const *),
                void *: sizeof(void *),
                struct pl: sizeof(struct pl),
                default: sizeof(void *)),
        (prm),
        _Generic((0) ? (tone) : (tone),
                _Bool: sizeof(int),
                char: sizeof(int),
                unsigned char: sizeof(unsigned int),
                short: sizeof(int),
                unsigned short: sizeof(unsigned int),
                int: sizeof(int),
                unsigned int: sizeof(unsigned int),
                long: sizeof(long),
                unsigned long: sizeof(unsigned long),
                long long: sizeof(long long),
                unsigned long long: sizeof(unsigned long long),
                float: sizeof(double),
                double: sizeof(double),
                char const *: sizeof(char const *),
                char *: sizeof(char *),
                void const *: sizeof(void const *),
                void *: sizeof(void *),
                struct pl: sizeof(struct pl),
                default: sizeof(void *)),
        (tone), 0)
@sreimers
Copy link
Member

This is a false positive and a needed workaround for some compilers.

@juha-h
Copy link
Contributor Author

juha-h commented Oct 22, 2024

So is your suggestion that I open an Android Studio ticket about this?

@sreimers
Copy link
Member

Here are the details:
#1110
https://gcc.gnu.org/legacy-ml/gcc/2016-02/msg00266.html

But since it's a gcc bug/workaround, I'm unsure if there is a real fix for clang-tidy.

@sreimers
Copy link
Member

Maybe a // NOLINT for clang-tidy is a option. but unsure how it have to look like and it works for this macro.

@juha-h
Copy link
Contributor Author

juha-h commented Oct 23, 2024

I created Android Studio ticket, but usually they don't lead to anything.

@juha-h
Copy link
Contributor Author

juha-h commented Nov 4, 2024

I got reply from Android Studio to my ticket:

I see the following in the macro definition:

_Generic((0) ? (prm) : (prm)

Which is a conditional operator, and it has prm on both sides. Clang-Tidy warning seems reasonable to me.

If I replace that line with:

_Generic((prm)

Then the Clang-Tidy warning disappears. I don't know what purpose that ternary operator serves.

So why can't the macro definition be changed like proposed?

@juha-h juha-h reopened this Nov 4, 2024
@sreimers
Copy link
Member

sreimers commented Nov 4, 2024

See GCC Bitfields bug: #1110

@sreimers sreimers transferred this issue from baresip/baresip Nov 4, 2024
@sreimers
Copy link
Member

sreimers commented Nov 4, 2024

You can try #1206

@juha-h
Copy link
Contributor Author

juha-h commented Nov 4, 2024

I'm on the road. Will try when somewhere.

@juha-h
Copy link
Contributor Author

juha-h commented Nov 4, 2024

#1206 helped and re_snprintf complaints are gone.

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 a pull request may close this issue.

2 participants