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

cppcheck warned about a comparison of pointers from different objects #1584

Merged
merged 1 commit into from
Mar 30, 2022

Conversation

smoe
Copy link
Contributor

@smoe smoe commented Feb 9, 2022

This may not be overly dramatical since hidden in a #debug conditional, but for the sake of clarity and to add src/emc/nml_intf to the whitelisted set of directories proposed in #1581, this change may be fine.

interpl.cc:88:36: error: Comparing pointers that point to different objects [comparePointers]
 ((void *) &temp_node.line_number) >
                                   ^
interpl.hh:26:9: note: Variable declared here.
    int line_number;  // line number it was on
        ^
interpl.cc:88:12: note: Address of variable taken here.
 ((void *) &temp_node.line_number) >
           ^
interpl.hh:37:7: note: Variable declared here.
 char commandbuf[MAX_NML_COMMAND_SIZE]; // the NML command;
      ^
interpl.cc:89:12: note: Address of variable taken here.
 ((void *) &temp_node.command.commandbuf)) {
           ^
interpl.cc:88:36: note: Comparing pointers that point to different objects
 ((void *) &temp_node.line_number) >

Comment on lines 88 to 89
((unsigned long) (void *) &temp_node.line_number) >
((unsigned long) (void *) &temp_node.command.commandbuf)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

From the language perspective, you cannot safely cast from void * to unsigned long as there is no guarantee that unsigned long will be capable of holding a value of pointer.

If you really want to cast void * to integer type please consider using uintptr_t type.

Cast to uintptr_t as pointed out by @dwrobel.

interpl.cc:88:36: error: Comparing pointers that point to different objects [comparePointers]
 ((void *) &temp_node.line_number) >
                                   ^
interpl.hh:26:9: note: Variable declared here.
    int line_number;  // line number it was on
        ^
interpl.cc:88:12: note: Address of variable taken here.
 ((void *) &temp_node.line_number) >
           ^
interpl.hh:37:7: note: Variable declared here.
 char commandbuf[MAX_NML_COMMAND_SIZE]; // the NML command;
      ^
interpl.cc:89:12: note: Address of variable taken here.
 ((void *) &temp_node.command.commandbuf)) {
           ^
interpl.cc:88:36: note: Comparing pointers that point to different objects
 ((void *) &temp_node.line_number) >
@smoe smoe force-pushed the compare_longs_not_void_pointers branch from 175639b to b172174 Compare February 10, 2022 11:56
@smoe
Copy link
Contributor Author

smoe commented Feb 10, 2022

Technically according to https://docs.oracle.com/cd/E19253-01/817-6223/chp-typeopexpr-2/index.html for 32 and 64 bit the unsigned long would have been fine but your suggestion (in the second table) reflects the semantics and is hence better. Have many thanks for spotting that! I rebased to master and updated as suggested.

@dwrobel
Copy link
Contributor

dwrobel commented Feb 10, 2022

64 bit the unsigned long

You were refering to "Solaris Dynamic Tracing Guide", while the c++ specification doesn't guarantee that, see the Minimum width N for long inttype, which only guarantees 32.

@andypugh andypugh merged commit f48dec0 into LinuxCNC:master Mar 30, 2022
@smoe smoe deleted the compare_longs_not_void_pointers branch March 30, 2022 22:42
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.

3 participants