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

logical operation on address of string constant warnings (is risk) #153

Open
diablodale opened this issue May 29, 2020 · 0 comments
Open
Labels

Comments

@diablodale
Copy link

Multiple warnings in min-api of "logical operation on address of string constant". This is due to comparing the addresses of two pointers; each of those pointers pointing to a block of char.

Setup

  • VS Community 2019 v16.6.0
  • max api commit 1c06b88fc30da0931b6dfa4a874bc081afe00926
  • min api commit 210d5da
  • min.jit.stencil.cpp from the min-devkit

Repo

  1. Remove any 'ignore warning' flags from your cmake harness
  2. Compile min.jit.stencil.cpp

Result

Build fails with:

[build] C:\repos-nobackup\min-api\include\c74_min_object_wrapper.h(242): error C2220: the following warning is treated as an error
[build] C:\repos-nobackup\min-api\include\c74_min_object_wrapper.h(785): note: see reference to function template instantiation 'void c74::min::wrapper_method_multitouch<min_class_type,c74::min::wrapper_message_name_mt_mouseenter>(c74::max::t_object *,c74::max::t_object *,c74::max::t_mouseevent *)' being compiled
[build]         with
[build]         [
[build]             min_class_type=jit_stencil
[build]         ]
[build] ..\min.jit.stencil.cpp(70): note: see reference to function template instantiation 'void c74::min::wrap_as_max_external<jit_stencil,0>(const char *,const char *,void *,min_class_type *)' being compiled
[build]         with
[build]         [
[build]             min_class_type=jit_stencil
[build]         ]
[build] C:\repos-nobackup\min-api\include\c74_min_object_wrapper.h(242): warning C4130: '==': logical operation on address of string constant
[build] C:\repos-nobackup\min-api\include\c74_min_object_wrapper.h(244): warning C4130: '==': logical operation on address of string constant
[build] C:\repos-nobackup\min-api\include\c74_min_object_wrapper.h(246): warning C4130: '==': logical operation on address of string constant
[build] C:\repos-nobackup\min-api\include\c74_min_object_wrapper.h(248): warning C4130: '==': logical operation on address of string constant
[build] C:\repos-nobackup\min-api\include\c74_min_object_wrapper.h(250): warning C4130: '==': logical operation on address of string constant
[build] C:\repos-nobackup\min-api\include\c74_min_object_wrapper.h(252): warning C4130: '==': logical operation on address of string constant
[build] ninja: build stopped: subcommand failed.

Expected

No warnings or errors, a clean build, and an mxe output.

Notes

This is a valid problem and should not be ignored. The lines of code are errant.

if (name == "mt_mouseenter")
  1. name is an auto variable (char *) that was derived from a struct containing a static const char created with MIN_WRAPPER_CREATE_TYPE_FROM_STRING()
  2. "mt_mouseenter" is also a static const char created inplace
  3. Comparing two pointers is bad.

It might by luck work. Why? Because some compilers with some options (like string pooling, COMDAT folder, etc.) will recognize: 1) the string bytes created with MIN_WRAPPER_CREATE_TYPE_FROM_STRING() and 2) the inplace bytes with "mt_mouseenter" are the same... and therefore have each refer to the same bytes in the executables data segment and therefore have the same pointer address. However, this is too risky. It depends on your specific compiler with specific compile options and that the compiler does the pooling and recognizes the two are the same.

The correct way to compare two C-style POD strings is to use strcmp(), convert them to std::string, etc.

@robtherich robtherich added the bug label Jan 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants