Confusing behaviour of pointers

Go To Last Post
20 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hey people,

 

I am having trouble with initialising these pointers. I have the following function (apologies in advance for the long variable names, they aren't very relevant)

 

struct __heaplist {
	struct _freelist *__flp;
	char *__malloc_heap_start;
	char *__malloc_heap_end;
	char *__brkval;
};

struct __heaplist *__hp1;
struct __heaplist *__hp2;

void heaps_init() {
	struct __freelist *__flp_temp = 0;

	__hp1->__flp = __flp_temp;
	__hp1->__malloc_heap_start = __malloc_heap_start;
	__hp1->__malloc_heap_end = __malloc_heap_start + 15;
	__hp1->__brkval = 0;

	__hp2->__flp = __flp_temp;
	__hp2->__malloc_heap_start = __hp1->__malloc_heap_end + __malloc_margin_mw;
	__hp2->__malloc_heap_end = 0;
	__hp2->__brkval = 0;
}

With `heaps_init()` being called right in the beginning of main().

 

The problem I am seeing is that printing out all __hp1 values right after they are initialised shows they are given the right values. But printing them out again after __hp2 is initialised shows they now have the same values as __hp2. This blows my mind, is this a quirk of pointers that I have misunderstood?

This topic has a solution.
Last Edited: Thu. Jan 11, 2018 - 10:38 PM
This reply has been marked as the solution. 
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

The problem is the two global pointers are not initialized - they point to undefined memory.  Either call malloc() first which in the embedded world is not really good practice, or allocated a "regular" structure somewhere and point to it.  Basically assign the __hp1 and __hp2 pointers to actual memory.

 

Something like this:

struct __heaplist {
	struct _freelist *__flp;
	char *__malloc_heap_start;
	char *__malloc_heap_end;
	char *__brkval;
};

struct __heaplist __hp1_allocated;
strcut __heaplist __hp2_allocated;

struct __heaplist *__hp1;
struct __heaplist *__hp2;

void heaps_init() {
	struct __freelist *__flp_temp = 0;

    __hp1 = &__hp1_allocated;   // assign the __hp1 pointer to some actual memory
	__hp1->__flp = __flp_temp;
	__hp1->__malloc_heap_start = __malloc_heap_start;
	__hp1->__malloc_heap_end = __malloc_heap_start + 15;
	__hp1->__brkval = 0;

    __hp2 = &__hp2_allocated;   // assign the __hp2 pointer to some actual memory
	__hp2->__flp = __flp_temp;
	__hp2->__malloc_heap_start = __hp1->__malloc_heap_end + __malloc_margin_mw;
	__hp2->__malloc_heap_end = 0;
	__hp2->__brkval = 0;
}

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I'll bite:  Which AVR8 model has enough SRAM (and flash) space such that a heap management mechanism can be justified?

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Interesting, your solution worked but I am not sure I understand it. If the pointers point to undefined memory, wouldn't it dump garbage data? Why does it follow __hp2?

 

@theusch I'll be utilising external RAM chips and a few other things related to my project. There are reasons for the logical separation of the memory :P

Last Edited: Wed. Jan 10, 2018 - 10:04 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

ThePageMan wrote:
If the pointers point to undefined memory, wouldn't it dump garbage data? Why does it follow __hp2?

Since the pointers as such are global variables they are both initialized to zero upon start. Nota bene! The pointers themselves are initialized to zero. This means that they point to the same thing. If you use one of the pointers and write something to what it points to and then use the other pointer so investigate the value it points to it will be the same value that was written.

As of January 15, 2018, Site fix-up work has begun! Now do your part and report any bugs or deficiencies here

No guarantees, but if we don't report problems they won't get much of  a chance to be fixed! Details/discussions at link given just above.

 

"Some questions have no answers."[C Baird] "There comes a point where the spoon-feeding has to stop and the independent thinking has to start." [C Lawson] "There are always ways to disagree, without being disagreeable."[E Weddington] "Words represent concepts. Use the wrong words, communicate the wrong concept." [J Morin] "Persistence only goes so far if you set yourself up for failure." [Kartman]

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

JohanEkdahl wrote:

The pointers themselves are initialized to zero.

 

Ah of course! https://en.wikipedia.org/wiki/Un...

 

It never clicked that 0 would still be a valid address. Two pointers pointing at the same thing and then I wonder why one changes the other. Thanks :)

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

@ThePageMan said:

It never clicked that 0 would still be a valid address.

It isn't.  NULL pointers (i.e., pointers whose value is zero) are typically used to indicate the pointer is, er, null (it doesn't point to anything). 

 

It's not clear from your comments that you are grasping the allocation aspect.  Declaring a pointer to a struct:

struct __heaplist *__hp1;

allocates two bytes which contain the address of an instance of the struct.  It does not allocate the 8 bytes for the struct.  That is why you must declare both the struct and the pointer to it (or use malloc() which, as already stated, is not a good solution):

// define the struct
struct __heaplist {
	struct _freelist *__flp;
	char *__malloc_heap_start;
	char *__malloc_heap_end;
	char *__brkval;
};

// declare an instance of the struct (allocates 8 bytes)
struct __heaplist __hp1_allocated;

// declare a pointer to the struct (allocates 2 byte)
struct __heaplist *__hp1 = &__hp1_allocated;

 

EDIT: corrected code.

Greg Muth

Portland, OR, US

Xplained/Pro/Mini Boards mostly

 

Last Edited: Thu. Jan 11, 2018 - 12:52 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Greg_Muth wrote:
It's not clear from your comments that you are grasping the allocation aspect.

It seems to be a common problem

 

allocates two bytes which contain the address of an instance of the struct.  It does not allocate the 8 bytes for the struct

Same issue only the other day: https://www.avrfreaks.net/comment...

 

 

#PointerAllocation

 

Top Tips:

  1. How to properly post source code - see: https://www.avrfreaks.net/comment... - also how to properly include images/pictures
  2. "Garbage" characters on a serial terminal are (almost?) invariably due to wrong baud rate - see: https://learn.sparkfun.com/tutorials/serial-communication
  3. Wrong baud rate is usually due to not running at the speed you thought; check by blinking a LED to see if you get the speed you expected
  4. Difference between a crystal, and a crystal oscillatorhttps://www.avrfreaks.net/comment...
  5. When your question is resolved, mark the solution: https://www.avrfreaks.net/comment...
  6. Beginner's "Getting Started" tips: https://www.avrfreaks.net/comment...
Last Edited: Thu. Jan 11, 2018 - 10:18 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Oh it's a good thing I decided to check this thread again. It seems marking a solution stops any new replies being sent to my email.

 

I see the distinction now yeah! You need to physically allocate the memory for the struct as the pointer is just... a pointer.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

ThePageMan wrote:
@theusch I'll be utilising external RAM chips and a few other things related to my project. There are reasons for the logical separation of the memory :P
Even with external memory:

theusch wrote:
Which AVR8 model has enough SRAM (and flash) space such that a heap management mechanism can be justified?
I guess I'd need to rephrase that to "AVR8 application".  But suit yourself.  Probably if I were to attack an app today that had "reasons", I'd probably jump ahead to a Cortex (M? A?) that already has stuff build in and faster clock and probably less $$$.

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

theusch wrote:
Which AVR8 model has enough SRAM (and flash) space such that a heap management mechanism can be justified?

mega2560?

 

theusch wrote:
But suit yourself.

Absolutely! We're not "style cops" here, are we? ;-)

As of January 15, 2018, Site fix-up work has begun! Now do your part and report any bugs or deficiencies here

No guarantees, but if we don't report problems they won't get much of  a chance to be fixed! Details/discussions at link given just above.

 

"Some questions have no answers."[C Baird] "There comes a point where the spoon-feeding has to stop and the independent thinking has to start." [C Lawson] "There are always ways to disagree, without being disagreeable."[E Weddington] "Words represent concepts. Use the wrong words, communicate the wrong concept." [J Morin] "Persistence only goes so far if you set yourself up for failure." [Kartman]

Last Edited: Thu. Jan 11, 2018 - 03:48 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I'm going to question the purpose of declaring __hp1 and __hp2 as pointers in the first place. Why not simply make them instances of struct __heaplist like this?

struct __heaplist __hp1;
struct __heaplist __hp2;

Then change "->" to "." when you access them.

 

This saves two bytes (for the pointers) and memory allocation overhead.

 

EDIT: there's no memory allocation overhead if __hp1 points to a static variable (there's overhead only if something like "malloc()" or "new" is used to allocate an instance of struct __heaplist). But there's still 2 bytes wasted to make __hp1 point to the static variable.

Last Edited: Thu. Jan 11, 2018 - 04:48 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

JohanEkdahl wrote:
We're not "style cops" here, are we? ;-)

laugh

 

Top Tips:

  1. How to properly post source code - see: https://www.avrfreaks.net/comment... - also how to properly include images/pictures
  2. "Garbage" characters on a serial terminal are (almost?) invariably due to wrong baud rate - see: https://learn.sparkfun.com/tutorials/serial-communication
  3. Wrong baud rate is usually due to not running at the speed you thought; check by blinking a LED to see if you get the speed you expected
  4. Difference between a crystal, and a crystal oscillatorhttps://www.avrfreaks.net/comment...
  5. When your question is resolved, mark the solution: https://www.avrfreaks.net/comment...
  6. Beginner's "Getting Started" tips: https://www.avrfreaks.net/comment...
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

We're not "style cops" here, are we? ;-)

Speaking of, you shouldn't use leading underscores in your names:

https://stackoverflow.com/questions/25090635/use-and-in-c-programs

"Experience is what enables you to recognise a mistake the second time you make it."

"Good judgement comes from experience.  Experience comes from bad judgement."

"Wisdom is always wont to arrive late, and to be a little approximate on first possession."

"When you hear hoofbeats, think horses, not unicorns."

"Fast.  Cheap.  Good.  Pick two."

"Read a lot.  Write a lot."

"We see a lot of arses on handlebars around here." - [J Ekdahl]

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

christop wrote:

I'm going to question the purpose of declaring __hp1 and __hp2 as pointers in the first place. Why not simply make them instances of struct __heaplist like this?

struct __heaplist __hp1;
struct __heaplist __hp2;

Then change "->" to "." when you access them.

 

I've converted the variables into a linked list connected together. I'd been meaning to do that for awhile and, if I'm not wrong, it needs to be a struct pointer?

 

joeymorin wrote:

We're not "style cops" here, are we? ;-)

Speaking of, you shouldn't use leading underscores in your names:

https://stackoverflow.com/questions/25090635/use-and-in-c-programs

 

This is a point I've been in a grey area about. I am changing the functionality of malloc.c itself, so I feel like consistency would be most appropriate here. And therefore underscores are used.

Last Edited: Thu. Jan 11, 2018 - 10:37 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

ThePageMan wrote:
a linked list

It is a common misconception, but note that it is not necessary (although it is common) for a linked-list to be dynamically allocated.

 

Especially as you seem to be trying to replace malloc() - it seems rather bizarre to have your replacement still rely upon it!

 

surprise

 

EDIT

 

typo

Top Tips:

  1. How to properly post source code - see: https://www.avrfreaks.net/comment... - also how to properly include images/pictures
  2. "Garbage" characters on a serial terminal are (almost?) invariably due to wrong baud rate - see: https://learn.sparkfun.com/tutorials/serial-communication
  3. Wrong baud rate is usually due to not running at the speed you thought; check by blinking a LED to see if you get the speed you expected
  4. Difference between a crystal, and a crystal oscillatorhttps://www.avrfreaks.net/comment...
  5. When your question is resolved, mark the solution: https://www.avrfreaks.net/comment...
  6. Beginner's "Getting Started" tips: https://www.avrfreaks.net/comment...
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

I am changing the functionality of malloc.c itself

Irrelevant:

  • All identifiers that begin with an underscore and either an uppercase letter or another underscore are always reserved for any use.
  • All identifiers that begin with an underscore are always reserved for use as identifiers with file scope in both the ordinary and tag name spaces.

(The section goes on to list specific identifiers and sets of identifiers reserved by certain standard headers.)

What this means is that for example, the implementation (either the compiler or a standard header) can use the name __FOO for anything it likes. If you define that identifier in your own code, your program's behavior is undefined. If you're "lucky", you'll be using an implementation that doesn't happen to define it, and your program will work as expected.

.

You can use an identifier like _foo as long as it's defined locally (not at file scope) -- but personally I find it much easier just to avoid using leading underscores at all.

 

This:

struct __heaplist {
	struct _freelist *__flp;
	char *__malloc_heap_start;
	char *__malloc_heap_end;
	char *__brkval;
};

struct __heaplist __hp1_allocated;
strcut __heaplist __hp2_allocated;

struct __heaplist *__hp1;
struct __heaplist *__hp2;

Breaks all of those rules.  Even if you dropped the second underscore:

struct _heaplist {
struct _heaplist _hp1_allocated;
strcut _heaplist _hp2_allocated;

struct _heaplist *_hp1;
struct _heaplist *_hp2;

... it would still break the second rule since the struct definition, the structs themselves,  and the pointers to them, are all defined at file scope.

"Experience is what enables you to recognise a mistake the second time you make it."

"Good judgement comes from experience.  Experience comes from bad judgement."

"Wisdom is always wont to arrive late, and to be a little approximate on first possession."

"When you hear hoofbeats, think horses, not unicorns."

"Fast.  Cheap.  Good.  Pick two."

"Read a lot.  Write a lot."

"We see a lot of arses on handlebars around here." - [J Ekdahl]

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

That's a fair argument. I've removed the leading underscores.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 2

Why don't you use typedef's? The code would be easier to read. (Much less use of "struct" all over the place!)

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Again, I am trying to stay consistent with the original malloc.c which seemed to forgo the use of typedef. I haven't checked the C standard for any explanation on it so I may be in the wrong but due to time constraints I am deeming it low priority :P