| View previous topic :: View next topic |
| Author |
Message |
chux
Joined: 24 Oct 2016
Posts: 3
|
| memcmp unexpected --> Pointers types do not match |
 Posted: Mon Oct 24, 2016 1:07 pm |
 |
|
In <string.h>, memcmp() has the definition
| Code: |
// <string.h>
signed int8 memcmp(void * s1,void *s2,size_t n)
// my code
unsigned foo(uint64_t *dest) {
uint64_t *dest0 = dest;
if (memcmp(&dest0[i], dest, sizeof(*dest)) == 0) {
|
Why does memcmp(&dest0[i], dest, sizeof(*dest)) generate the warning "Pointer types does not match"?
The below quiets the warning, yet in C, all data pointers match a `void *`., so why the warning?
| Code: |
if (memcmp(&dest0[i], (void*) dest, sizeof(*dest)) == 0) {
|
Version 5.060 |
|
 |
Ttelmah
Joined: 11 Mar 2010
Posts: 19559
|
|
| Posted: Mon Oct 24, 2016 1:50 pm |
|
|
CCS, doesn't actually have a void * type in the sense of a 'undefined' pointer type that will match any type in use....
You have to cast the pointers to the required type, to get rid of the warning.
In the past CCS did not give a warning. This was added a few versions ago, and I have moaned at CCS, and pointed out that if they are going to be strict on this, they ought to implement the void * ability. |
|
|
jeremiah
Joined: 20 Jul 2010
Posts: 1358
|
|
| Posted: Mon Oct 24, 2016 2:15 pm |
|
|
I tried compiling with PCD 5.061 and got no warnings.
Test Code:
| Code: |
#case
#include "pic.h"
#include "lcd.h"
#include <stdint.h>
#include <string.h>
unsigned foo(uint64_t *dest) {
uint64_t *dest0 = dest;
unsigned int16 i = 0;
if (memcmp(&dest0[i], dest, sizeof(*dest)) == 0) {
return 1;
}
return 0;
}
void main(void){
lcd_init();
delay_ms(100);
lcd_powerOn();
lcd_clear();
lcd_printf(" ^ ^ \r\n");
lcd_printf(" <> \r\n");
lcd_printf(" <==> ");
printf("Hello Jeremiah!\r\n");
while(TRUE);
}
#include "lcd.c"
|
As a note, your third parameter seems odd. taking the sizeof of a pointer is not correct usually. |
|
|
chux
Joined: 24 Oct 2016
Posts: 3
|
|
| Posted: Mon Oct 24, 2016 3:34 pm |
|
|
Uploaded to the recent 5.064 version and the problem went away - yeah!
Thanks
Concerning "your third parameter seems odd. taking the sizeof of a pointer is not correct usually." Hope you do not think the code was taking the sizeof of a pointer. `sizeof dest` is the size of a pointer. The below code takes the size of the type referenced by a pointer. This style of coding, used for decades, is simpler to maintain and code than `sizeof(some_type)`
| Code: |
uint64_t *dest;
sizeof(*dest)
|
|
|
|
jeremiah
Joined: 20 Jul 2010
Posts: 1358
|
|
| Posted: Mon Oct 24, 2016 4:37 pm |
|
|
I wasn't very clear in my wording. You almost *never* do
As it will always return the same answer regardless of how long your buffer is.
As an example, take the example code:
| Code: |
#case
#include "pic.h"
#include "lcd.h"
#include <stdint.h>
void test_sizeof(uint64_t * dest){
printf("sizeof(*dest) = %u\r\n", sizeof(*dest));
}
void main(void){
uint64_t test_buffer[50];
lcd_init();
delay_ms(100);
lcd_powerOn();
lcd_clear();
lcd_printf(" ^ ^ \r\n");
lcd_printf(" <> \r\n");
lcd_printf(" <==> ");
printf("Hello Jeremiah!\r\n");
test_sizeof(test_buffer);
while(TRUE);
}
#include "lcd.c"
|
It merrily prints out:
| Quote: |
Hello Jeremiah!
sizeof(*dest) = 8
|
Because taking sizeof() on a dereferenced pointer will always give the size of a single element.
You may indeed only want memcmp to only compare one element (8 bytes in your case), I was just saying that it isn't a very normal thing (usually for single value comparisons, you just compare the dereferenced pointers as memcmp is overkill for a single element). You see a lot of people mistakenly think that sizeof(*dest) gives you the size of the whole buffer, when it doesn't. I don't know your background, so I was tossing that out there. I've been programming for decades as well. Now I am feeling old! |
|
|
chux
Joined: 24 Oct 2016
Posts: 3
|
|
| Posted: Mon Oct 24, 2016 5:28 pm |
|
|
Fair enough fellow seasoned programmer.
It does look a bit odd. as code could have been `if (dest0[i] == dest) {`. It was coded as `if (memcmp(&dest0[i], dest, sizeof(*dest)) == 0)` as that was significantly few instructions at one point in the past. Also its baseline was PIC16 which lacks 64-bit type.
Reviewing the resultant code today shows them about the same, and as you well pointed out, a single value compare makes more sense. Time to update the code. Thank you for the review. |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19559
|
|
| Posted: Tue Oct 25, 2016 12:32 am |
|
|
That's good. I hadn't spotted that the latest releases have actually changed this warning so it behaves more sensibly again. 
A 'silent' improvement.... |
|
|
|
FAQ 
Search
Register
Profile 
Log in to check your private messages 
Log in
