15 Mar, 2013, arholly wrote in the 1st comment:
Votes: 0
Hello:
I have a question about something and I don't understand. I decided to use one of the static code analyzers, and me being a inexperienced programmer, don't understand one of the results. It told me Size of pointer 'pReset' used instead of size of its data. What does that mean and how can I fix it? If someone can give me a plain English explanation, I'd greatly appreciate it. I've got several instances of those I'd like to fix.
RESET_DATA *new_reset_data( void )
{
RESET_DATA *pReset;

ALLOC_DATA(pReset, RESET_DATA, 1);
memset(pReset, 0, sizeof(pReset));

pReset->next = NULL;
pReset->command = 'X';
pReset->arg1 = 0;
pReset->arg2 = 0;
pReset->arg3 = 0;

return pReset;
}
15 Mar, 2013, salindor wrote in the 2nd comment:
Votes: 0
Can you post what ALLOC_DATA is?

If you do sizeof (RESET_DATA *) it will return the size of a pointer which on a 64bit machine is 8. Where as if you do sizeof(RESET_DATA) it will return the size of the structure. Most people mean the later and not the former


Salindor
15 Mar, 2013, arholly wrote in the 3rd comment:
Votes: 0
Sure I can.

#define ALLOC_DATA(pointer, datatype, elements)   \
do { \
if (!((pointer) = (datatype *) calloc ((elements), sizeof(datatype)))) \
{ \
logfmt("Unable to calloc new data from file: %s function: %s, line: %d\t *** ABORTING ***", __FILE__, __FUNCTION__, __LINE__); \
abort(); \
} \
} \
while(0)

#define PURGE_DATA(data) \
do { \
if(data) \
{ \
free((void *)data); \
} \
else { \
if(logFail) { \
log_string((char *)Format("clear_free_lists: unable to purge_data as it was NULL from: %s %s %d", __FILE__, __FUNCTION__, __LINE__)); \
} \
} \
data = NULL; \
} while(0)
15 Mar, 2013, salindor wrote in the 4th comment:
Votes: 0
oh wait shoot, line 6. of your original code. it should read
memset(pReset, 0, sizeof(RESET_DATA ));


basically your only clearing the first 4 or 8 bytes of the structure (depending on the machine its targeting) instead of the whole thing
15 Mar, 2013, arholly wrote in the 5th comment:
Votes: 0
OK. So that was the problem. Being not very good at coding, I was not sure what exactly it was telling me. Thank you very much for explaining it plainly.
15 Mar, 2013, Rarva.Riendf wrote in the 6th comment:
Votes: 0
>It told me Size of pointer 'pReset' used instead of size of its data.

RESET_DATA *pReset <– pReset is indeed a pointer (use of * in declaration). As a pointer its size only depends on wich kind of cpu you are using. (16 32 64 bit or even more exotic stuff)

It is just an adress somewhere in memory where the data is, wich reserved size is determined by how it was declared (RESET_DATA in this case)
You shoudl read a good didactic C course . Once you are more confident with all the * [] & syntax (and understand what it actually does in memory), it gets a lot better…(still annoying as hell to use though…)
15 Mar, 2013, Vigud wrote in the 7th comment:
Votes: 0
Think of it this way:
RESET_DATA *pReset;

sizeof will tell you the size of the type of an expression…
sizeof 10000 <– what type did it get? What is the type of 10000?
sizeof pReset <– what is the type of pReset?
sizeof *pReset <– what is the type of *pReset?

sizeof will tell you the size of a type (requires parenthesis)
sizeof (RESET_DATA) <– what is the type?

I understand you're making novice mistakes, but the code you inherited is very likely already bad.

ALLOC_DATA() uses calloc(), which initializes allocated space to all-bits-zero. memset(pReset, 0, sizeof(RESET_DATA )); sets all bits to zero. The assignments finally set the struct members to proper "clear values". So at least the memset() is absolutely unnecessary. I'd go further and redefine ALLOC_DATA() to use malloc(), but you probably shouldn't, since doing so could break things in other places of the source-code you have.

Below are a few bad things I noticed in the small code fragments that you pasted in this thread. This is nit-picking, so if you're not interested in learning a thing or two, just ignore the rest of this message.

Quote
= (datatype *) calloc(…)

Don't cast malloc() in C.

Quote
memset(pReset, 0, sizeof(RESET_DATA ));

That method of "clearing" allocated memory for objects is wrong. Use either of these two:
#include <stdlib.h>

struct test {
int a;
struct {
int c;
char *d;
unsigned char x;
} b;
};

static const struct test zero_test;

int main(void) {
/* clearing automatic objects is the easiest */
struct test w = {0};

/* allocated space is a little bit trickier */
struct test *t = malloc(sizeof *t);
*t = zero_test; /* C89 */
*t = (struct test) { 0 }; /* C99 */

return t->b.x + w.b.x;
}


Quote
sizeof(RESET_DATA)

That makes as much sense as always writing return(a); return(b); etc. Neither return nor sizeof is a function. sizeof requires parenthesis only when you give it a type. See my example above, line 19.
16 Mar, 2013, quixadhal wrote in the 8th comment:
Votes: 0
Or, you could just NOT to the redundant memset() at all, since you're already using calloc(), which does that FOR you.
16 Mar, 2013, arholly wrote in the 9th comment:
Votes: 0
Vigud said:
<Snipe Explanation>
I understand you're making novice mistakes, but the code you inherited is very likely already bad.

ALLOC_DATA() uses calloc(), which initializes allocated space to all-bits-zero. memset(pReset, 0, sizeof(RESET_DATA )); sets all bits to zero. The assignments finally set the struct members to proper "clear values". So at least the memset() is absolutely unnecessary. I'd go further and redefine ALLOC_DATA() to use malloc(), but you probably shouldn't, since doing so could break things in other places of the source-code you have.

Thanks Vigud. I'm definitely making novice mistakes and the code I inherited was very bad. It was not anything close to stable. It's much better now, but still not great. I'm trying to get it better. :) OK, so I removed the unnecessary memset from that location and the others which generated similar errors. That got rid of the messages the analyzer was kicking out. Thank you for the explanation and suggestions (you and the others who said the same such as quixadhal and someone else suggested).

Vigud said:
Below are a few bad things I noticed in the small code fragments that you pasted in this thread. This is nit-picking, so if you're not interested in learning a thing or two, just ignore the rest of this message.

Quote
= (datatype *) calloc(…)

Don't cast malloc() in C.

Quote
memset(pReset, 0, sizeof(RESET_DATA ));

That method of "clearing" allocated memory for objects is wrong. Use either of these two:
#include <stdlib.h>

struct test {
int a;
struct {
int c;
char *d;
unsigned char x;
} b;
};

static const struct test zero_test;

int main(void) {
/* clearing automatic objects is the easiest */
struct test w = {0};

/* allocated space is a little bit trickier */
struct test *t = malloc(sizeof *t);
*t = zero_test; /* C89 */
*t = (struct test) { 0 }; /* C99 */

return t->b.x + w.b.x;
}


Quote
sizeof(RESET_DATA)

That makes as much sense as always writing return(a); return(b); etc. Neither return nor sizeof is a function. sizeof requires parenthesis only when you give it a type. See my example above, line 19.

OK, so for the malloc suggestion. Clearly I'm not understanding since calloc was used and that referred to malloc. Is it the same principle?
16 Mar, 2013, Vigud wrote in the 10th comment:
Votes: 0
calloc() is a malloc() that sets all bits of the area to zero.
16 Mar, 2013, quixadhal wrote in the 11th comment:
Votes: 0
calloc() and malloc() are both high level memory allocators that (likely) invoke alloca() and/or sbrk()… They do the same thing, but calloc() is additionally guarenteed to zero out the memory range returned. There are real-time cases where you don't want the extra cycles spent clearing the memory, but a MUD isn't one of them. :)
16 Mar, 2013, Rarva.Riendf wrote in the 12th comment:
Votes: 0
Quote
There are real-time cases where you don't want the extra cycles spent clearing the memory, but a MUD isn't one of them. :)


Indeed. I think calloc has a drawback though, as if it will make your code more stable. It can make logical/algorithmic bugs run a lot longer without being detected.
Use calloc is the best practice if you unit test your code. If you are too lazy(dont do that in a professional code!)/cannot be bothered as it is a hobby to unit test your code, malloc will makes some bugs rise faster.

Though I have seen strange things using cygwin, as on Windows it seems that calloc or not, everything new memory allocated is set to 0. Very annoying. (at least on my settings when I was using it)
17 Mar, 2013, salindor wrote in the 13th comment:
Votes: 0
Rarva.Riendf said:
Quote
There are real-time cases where you don't want the extra cycles spent clearing the memory, but a MUD isn't one of them. :)


Indeed. I think calloc has a drawback though, as if it will make your code more stable. It can make logical/algorithmic bugs run a lot longer without being detected.
Use calloc is the best practice if you unit test your code. If you are too lazy(dont do that in a professional code!)/cannot be bothered as it is a hobby to unit test your code, malloc will makes some bugs rise faster.

Though I have seen strange things using cygwin, as on Windows it seems that calloc or not, everything new memory allocated is set to 0. Very annoying. (at least on my settings when I was using it)


You can get around that issue by designing your algorithms so a value of 0 is considered undefined or error or whatnot. Then using calloc is a good idea because it guarantees regardless of your compiler and their settings uninitialized variables will be zeroed out and your enumerations will tell you that something is wrong. Though this trick won't help when you add a new variable and forgot to initialize it (if it should be initialized to anything but 0).

Salindor
17 Mar, 2013, Tyche wrote in the 14th comment:
Votes: 0
quixadhal said:
calloc() and malloc() are both high level memory allocators that (likely) invoke alloca() and/or sbrk()…

Not likely alloca(), as it allocates memory on the stack.


Rarva.Riendf said:
Indeed. I think calloc has a drawback though, as if it will make your code more stable. It can make logical/algorithmic bugs run a lot longer without being detected.
Use calloc is the best practice if you unit test your code. If you are too lazy(dont do that in a professional code!)/cannot be bothered as it is a hobby to unit test your code, malloc will makes some bugs rise faster.


IMNSHO, it's just the opposite. Setting allocated memory to some initial value, zero or some magic number makes it easier to debug an application.

Rarva.Riendf said:
Though I have seen strange things using cygwin, as on Windows it seems that calloc or not, everything new memory allocated is set to 0. Very annoying. (at least on my settings when I was using it)


Cygwin uses Newlib. Newlib uses Doug Lea's malloc(). It uses VirtualAlloc() to acquire it's working set of pages.
VirtualAlloc() returns pages all-bits zeroed. However, this does not mean that malloc() returns all-bits zeroed at all.
It's easy to demonstrate that it doesn't:
jlambert@ATLAS ~
$ cat tm.c
#include <stdlib.h>
#include <stdio.h>

void dumpmem(char *p, char *msg) {
char i;
printf("%s\n",msg);
for (i=0;i<100;i++) {
if (!(i % 16)) printf("\n");
printf("%02hhx ",*(p+i));
}
printf("\n");
}

int main() {
char *p;
char i;

p = malloc(100*sizeof(char));
dumpmem(p, "Memory after 1st malloc:");

for (i=0;i<100;i++) *(p+i) = i;
dumpmem(p, "Memory after initializing:");

printf("Free the memory\n");
free(p);

p = malloc(100*sizeof(char));
dumpmem(p, "Memory after 2nd malloc:");

return 0;
}
jlambert@ATLAS ~
$ gcc tm.c

jlambert@ATLAS ~
$ ./a
Memory after 1st malloc:

00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00
Memory after initializing:

00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f
10 11 12 13 14 15 16 17 18 19 1a 1b 1c 1d 1e 1f
20 21 22 23 24 25 26 27 28 29 2a 2b 2c 2d 2e 2f
30 31 32 33 34 35 36 37 38 39 3a 3b 3c 3d 3e 3f
40 41 42 43 44 45 46 47 48 49 4a 4b 4c 4d 4e 4f
50 51 52 53 54 55 56 57 58 59 5a 5b 5c 5d 5e 5f
60 61 62 63
Free the memory
Memory after 2nd malloc:

4c d6 23 61 4c d6 23 61 08 09 0a 0b 0c 0d 0e 0f
10 11 12 13 14 15 16 17 18 19 1a 1b 1c 1d 1e 1f
20 21 22 23 24 25 26 27 28 29 2a 2b 2c 2d 2e 2f
30 31 32 33 34 35 36 37 38 39 3a 3b 3c 3d 3e 3f
40 41 42 43 44 45 46 47 48 49 4a 4b 4c 4d 4e 4f
50 51 52 53 54 55 56 57 58 59 5a 5b 5c 5d 5e 5f
68 00 00 00

malloc() under Cygwin has always operated this way.*
17 Mar, 2013, Rarva.Riendf wrote in the 15th comment:
Votes: 0
Quote
IMNSHO, it's just the opposite. Setting allocated memory to some initial value, zero or some magic number makes it easier to debug an application.


To debug yes I totally agree on that. I was talking about finding the bug in the first place. Random values used tends to make weirder behaviour than initialised to a 'sane one'. (kind of an automatic fuzzy test unit ;p)
If possible I initialize to value that I know should not be used in order to crash/fire a log when used.

Quote
malloc() under Cygwin has always operated this way.*


Nice to know. Never bothered to find the reason why I had evertything zeroed when I was debugging something. Only found out because I was coding mostly on Windows and the code 'worked' but crashed when testing under Linux. (kinda why I think it is better to have random values when you are obviously lazy coding ;p)

Now I have a better computer so I totally code and test under Linux. (Still using VM).
17 Mar, 2013, Runter wrote in the 16th comment:
Votes: 0
This is off topic, but I suggest in the future making the title of the thread something more useful than "i just don't understand." Try to make it useful for someone coming to the thread before they open it.
17 Mar, 2013, arholly wrote in the 17th comment:
Votes: 0
Gotcha. Will do.
17 Mar, 2013, Tyche wrote in the 18th comment:
Votes: 0
Rarva.Riendf said:
Nice to know. Never bothered to find the reason why I had evertything zeroed when I was debugging something. Only found out because I was coding mostly on Windows and the code 'worked' but crashed when testing under Linux. (kinda why I think it is better to have random values when you are obviously lazy coding ;p)

Now I have a better computer so I totally code and test under Linux. (Still using VM).


I ran it on several Linuxes. The behavior of sbrk() is identical to Windows VirtualAlloc().
That is both OSes hand out memory pages that are zeroed out.
It occurred to me that it would be a very bad thing if they did not.
17 Mar, 2013, Rarva.Riendf wrote in the 19th comment:
Votes: 0
Tyche said:
It occurred to me that it would be a very bad thing if they did not.


I do not think so: the behaviour is explicit: malloc is not supposed to zero out the memory. You can have memory to zero if you go to some place that was never allocated. Maybe some security mecanism of the OS (dunno if it exist just imagination here) zero out everything once it is freed, so everytim you allocate memory you will have zero etc.
I suppose there was some mecanism that were involved in the fact I always had zero when I used malloc (for the first time indeed) on Windows with cygwin, but the same code did not react at all the same under Linux. Dunno if it would stilll happen, and actually cannot be bothered to find out, as since the basics is to not take undefinied behaviour for granted (like using sprintf(buf, "%s%s", buf, "whatever"))).

But I am sure it behaved differently at that time, since it was the reason I found out the bug in the first place. (it was on a 32bit machine, P4 m)
17 Mar, 2013, Tyche wrote in the 20th comment:
Votes: 0
Both the newer Linux kernels and Windows kernel zero-out memory pages before handing them over to a process.
It would be a nasty security hole to hand out discarded pages from other processes without scrubbing them in some way.
This has nothing to do with C, or the requirements of malloc/calloc. The random garbage one might find on a block from malloc() isn't really random at all.
It all a creation of your own process.

Oddly enough, malloc() in the Cygwin newlib implementation detects memory corruption long before the OpenSuse glibc implementation I tested.

To demonstrate:
#include <stdlib.h>
#include <stdio.h>

void dumpmem(char *p, char *msg) {
char i;
printf("%s\n",msg);
for (i=0;i<100;i++) {
if (!(i % 16)) printf("\n");
printf("%02hhx ",*(p+i));
}
printf("\n");
}

int main() {
char *p, *q;
char i;

p = malloc(100*sizeof(char));
dumpmem(p, "Memory after 1st malloc:");

for (i=0;i<100;i++) *(p+i) = i;
dumpmem(p, "Memory after initializing:");

printf("Free the memory\n");
free(p);
dumpmem(p, "Dump memory after free:");

for (i=0;i<100;i++) *(p+i) = i;
dumpmem(p, "Reinitialize freed memory:");

printf("Core dumps here on 2nd malloc because\nwe've corrupted the magic bytes in our memory manager\n");
q = malloc(100*sizeof(char));
dumpmem(q, "Memory after 2nd malloc:");

return 0;
}


Cygwin malloc()
Memory after 1st malloc:

00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00
Memory after initializing:

00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f
10 11 12 13 14 15 16 17 18 19 1a 1b 1c 1d 1e 1f
20 21 22 23 24 25 26 27 28 29 2a 2b 2c 2d 2e 2f
30 31 32 33 34 35 36 37 38 39 3a 3b 3c 3d 3e 3f
40 41 42 43 44 45 46 47 48 49 4a 4b 4c 4d 4e 4f
50 51 52 53 54 55 56 57 58 59 5a 5b 5c 5d 5e 5f
60 61 62 63
Free the memory
Dump memory after free:

4c d6 23 61 4c d6 23 61 08 09 0a 0b 0c 0d 0e 0f
10 11 12 13 14 15 16 17 18 19 1a 1b 1c 1d 1e 1f
20 21 22 23 24 25 26 27 28 29 2a 2b 2c 2d 2e 2f
30 31 32 33 34 35 36 37 38 39 3a 3b 3c 3d 3e 3f
40 41 42 43 44 45 46 47 48 49 4a 4b 4c 4d 4e 4f
50 51 52 53 54 55 56 57 58 59 5a 5b 5c 5d 5e 5f
68 00 00 00
Reinitialize freed memory:

00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f
10 11 12 13 14 15 16 17 18 19 1a 1b 1c 1d 1e 1f
20 21 22 23 24 25 26 27 28 29 2a 2b 2c 2d 2e 2f
30 31 32 33 34 35 36 37 38 39 3a 3b 3c 3d 3e 3f
40 41 42 43 44 45 46 47 48 49 4a 4b 4c 4d 4e 4f
50 51 52 53 54 55 56 57 58 59 5a 5b 5c 5d 5e 5f
60 61 62 63
Core dumps here on 2nd malloc because
we've corrupted the magic bytes in our memory manager
Aborted (core dumped)


OpenSuse malloc()
Memory after 1st malloc:

00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00
Memory after initializing:

00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f
10 11 12 13 14 15 16 17 18 19 1a 1b 1c 1d 1e 1f
20 21 22 23 24 25 26 27 28 29 2a 2b 2c 2d 2e 2f
30 31 32 33 34 35 36 37 38 39 3a 3b 3c 3d 3e 3f
40 41 42 43 44 45 46 47 48 49 4a 4b 4c 4d 4e 4f
50 51 52 53 54 55 56 57 58 59 5a 5b 5c 5d 5e 5f
60 61 62 63
Free the memory
Dump memory after free:

00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f
10 11 12 13 14 15 16 17 18 19 1a 1b 1c 1d 1e 1f
20 21 22 23 24 25 26 27 28 29 2a 2b 2c 2d 2e 2f
30 31 32 33 34 35 36 37 38 39 3a 3b 3c 3d 3e 3f
40 41 42 43 44 45 46 47 48 49 4a 4b 4c 4d 4e 4f
50 51 52 53 54 55 56 57 58 59 5a 5b 5c 5d 5e 5f
60 61 62 63
Reinitialize freed memory:

00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f
10 11 12 13 14 15 16 17 18 19 1a 1b 1c 1d 1e 1f
20 21 22 23 24 25 26 27 28 29 2a 2b 2c 2d 2e 2f
30 31 32 33 34 35 36 37 38 39 3a 3b 3c 3d 3e 3f
40 41 42 43 44 45 46 47 48 49 4a 4b 4c 4d 4e 4f
50 51 52 53 54 55 56 57 58 59 5a 5b 5c 5d 5e 5f
60 61 62 63
Core dumps here on 2nd malloc because
we've corrupted the magic bytes in our memory manager
Memory after 2nd malloc:

00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f
10 11 12 13 14 15 16 17 18 19 1a 1b 1c 1d 1e 1f
20 21 22 23 24 25 26 27 28 29 2a 2b 2c 2d 2e 2f
30 31 32 33 34 35 36 37 38 39 3a 3b 3c 3d 3e 3f
40 41 42 43 44 45 46 47 48 49 4a 4b 4c 4d 4e 4f
50 51 52 53 54 55 56 57 58 59 5a 5b 5c 5d 5e 5f
60 61 62 63


Note the OpenSuse implementation doesn't crash, while Cygwin crashes immediately.
The reason is Cygwin's malloc/free heap manager actually marks up the freed memory block with information.
Which malloc() is better suited to finding memory corruption closer to the source of the problem?
Random Picks
0.0/21