Variable lifetime across functions

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

This is probably a fairly basic C question, but so far I've not found an answer to it and I'm relearning C after having not touched it for 20 or so years. 

 

The code I'm building needs a queue which I am representing as a circular buffer.  I therefore wrote a set of routines for operating on the buffer - enqueue, dequeue, etc.  The routines live in their own files, Queue.c and Queue.h, for better data abstraction.  I originally wrote this to operate on a single buffer, but now realize I may need more than one in my code.  So the routines need to take the buffer itself as an argument (in C++ or other OOP language this would be easily done by making the queue a class and instantiating multiple copies as needed, but I'm using straight C).

 

My concern comes from allocating space to the buffer itself.  I end up with pointers to structs which contain pointers to arrays, and I'm having trouble making sure I'm doing it right, especially in terms of memory lifetime.   Here's the relevant parts of the code:

 

The buffer is defined, in Queue.h, as a struct:

typedef struct
{
	uint8_t *que;

	//items go in through tail, come out through head
	uint8_t *qhead;  //points at oldest bit
	uint8_t *qtail;  //points at newest bit
	uint8_t *qstart; //start of queue
	uint8_t *qstop;  //end of queue


} queue_struct;

 

in my Main.c I statically instantiate a copy of the struct (it's marked volatile as there's a chance that it will need to be accessed from an interrupt:

static volatile queue_struct PacketQue;

The queue is set up with the following code (which lives in Queue.c):

static void QueueSetupDynamic(queue_struct *queue, uint16_t length)
{
	queue->que = malloc( length * sizeof( uint8_t ) );
	
	queue->qhead = queue->que;
	queue->qtail = queue->que;
	queue->qstart = (uint8_t *) queue->que;
	queue->qstop = (uint8_t *) queue->que + length -1;
}

This is called from Main.c with an explicit cast (MAXQLEN is #defined, in this case as 512):

	QueueSetup( ( queue_struct * ) &PacketQue, MAXQLEN );  //explicit cast to non-volatile

I believe this should work (it compiles, and seems to run correctly in the simulator, haven't tried to deploy it to the chip yet).  I also believe that by making PacketQue volatile, it will be treated that way if called from an interrupt, even though there's the cast at QueueSetup.  If I'm wrong in this, please let me know.

 

Now, the real question is that I'd really like to avoid using malloc.  All the best practices guides I've seen for embedded programming caution against it, and it seems like there shouldn't be a need for it - the size is fixed, by MAXQLEN, at compile-time.  But I can't figure out how to allocate the space.

My initial thought was something like this:
 

static void QueueSetupStatic(queue_struct *queue)
{
	uint8_t temp[MAXQLEN];
	queue->que = temp;
	
	queue->qhead = queue->que;
	queue->qtail = queue->que;
	queue->qstart = (uint8_t *) queue->que;
	queue->qstop = (uint8_t *) queue->que + MAXQLEN -1;
	
}

But I believe that the compiler will stop protecting that chunk of memory once "temp" goes out of scope.  I can declare it static, but that seems cludgy - it's not really a static variable of the function.

 

I can also declare a static array in Main.c, but again, that seems inelegant - data abstraction principles suggest that it belongs in Queue.c/ somewhere, not in Main.c

 

Ideas much appreciated.

This topic has a solution.
Last Edited: Mon. Mar 26, 2018 - 10:28 PM
This reply has been marked as the solution. 
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

The easy solution is to make your temp[] array a part of your que_struct type.

If you then declare a static object of your que_struct it will have infite lifetime.

 

This is all much cleaner if it is done in C++.

There is also not a good reason to do it in C instead of C++.

If you are interested in learning C++, then learning C first and learning C++ as a superset of C is not the best way.

 

If you want to do it in C, then you can get somewhat near the functionality of C++ by using a separate file for your circular buffer.

Functions which are declared as static in that file are somewhat equivalent to "private" functions in C++.

If you expose functions by putting the prototypes in the header file they are somewhat equivalent to "public".

Same for data and pointers.

 

Getting all the pointers exactly right in the implementation of a circular buffer is quite error prone.

It is a good exercise to learn C, but if you just want a working buffer it is probably a lot easier to use some existing and debugged code.

If you want to use this as an excersize in C, then I highly reccomend to develop your code on a PC.

You can then easily use printf to print your pointers and intermediate values to a terminal window while debugging.

 

The "volatile" keyword inhibits a lot of compiler optimisations.

It should not be used for the whole stucture, but only for the pointers which are actually used in the interrupt.

As you say you are a beginner with C, using pointers is often condidered "difficult".

You can easily avoid them by declaring your head and tail as simple uint8_t variables and use them as indices in the buffer array.

For a C compiler's point of view there is very little to no difference between arrays with indices or pointers.

When using indices there is also an extra trick you can use if your array has a size of 2^^x.

 

++index &= 0x0f;   // This increments "index" and limitits it to a value between 0 and 15.

Doing magic with a USD 7 Logic Analyser: https://www.avrfreaks.net/comment/2421756#comment-2421756

Bunch of old projects with AVR's: http://www.hoevendesign.com

Last Edited: Mon. Mar 26, 2018 - 06:53 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

To avoid using malloc(), allocate the buffers at build time instead of run time.

 

static void QueueSetupDynamic(queue_struct *queue,
    uint8_t *buffer, uint16_t length)
{
	queue->que = buffer;

	queue->qhead = queue->que;
	queue->qtail = queue->que;
	queue->qstart = (uint8_t *) queue->que;
	queue->qstop = (uint8_t *) queue->que + length -1;
}

main()
{
    uint8_t some_buffer[BUFFER_LENGTH];

    QueueSetupDynamic(&some_queue, some_buffer, BUFFER_LENGTH)
}

 

EDIT: I used poor wording as local variables are actually allocated on the stack at run time.  The lengths are known at build time however.

Greg Muth

Portland, OR, US

Xplained/Pro/Mini Boards mostly

 

Make Xmega Great Again!

 

Last Edited: Mon. Mar 26, 2018 - 06:47 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Paulvdh wrote:

The easy solution is to make your temp[] array a part of your que_struct type.

If you then declare a static object of your que_struct it will have infite lifetime.

 

Do you mean something like:

 

typedef struct
{
	uint8_t que[MAXQUE]; <-- CHANGE

	//items go in through tail, come out through head
	uint8_t *qhead;  //points at oldest bit
	uint8_t *qtail;  //points at newest bit
	uint8_t *qstart; //start of queue
	uint8_t *qstop;  //end of queue


} queue_struct;

?

 

That's be perfect, but I wasn't sure it'd work. 

 

The rest of the pointer stuff doesn't bother me too much - I had it all working, with a single buffer.  The buffer itself was a static and volatile array defined in the queue.c file, the other 4 elements of the struct were also static (and volatile) variables in the same file, and everything worked perfectly.  The actual circular buffer is trivial, that aspect of the pointer manipulation didn't cause much problem.  And yes, I modeled my code on examples from the web (written in C++, ironically).

 

It's the need to pass around a pointer to the whole buffer struct that made things complicated.  Which is ok, that's how one learns. :)

 

I used C++ as much as I used C, back in the day, but from everything I've read here, C seems a better plan for AVR programming - way more support.

 

Thanks!

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

Yep that is what I mean.

Are you familiar with github?

I did a little search there for a ringbuffer and got > 400 results.

Then I narrowed my search to only C programs and still had > 100 results.

https://github.com/search?l=C&q=circular+buffer&type=Repositories&utf8=%E2%9C%93

 

Then I thought that the 2nd from above could be a good example because of it's title: "Simple ringbuffer for Embedded systems"

https://github.com/AndersKaloer/Ring-Buffer

 

It seems to be very near to what you want to implement.

It has a structure with an array and 2 indices for head and tail.

struct ring_buffer_t {
  /** Buffer memory. */
  char buffer[RING_BUFFER_SIZE];
  /** Index of tail. */
  ring_buffer_size_t tail_index;
  /** Index of head. */
  ring_buffer_size_t head_index;
};

If you don't like this example, there are 113 others to select from.

Have fun.

Doing magic with a USD 7 Logic Analyser: https://www.avrfreaks.net/comment/2421756#comment-2421756

Bunch of old projects with AVR's: http://www.hoevendesign.com

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

I'd just use Dean's ringbuffer from LUFA. One .h file and that's it

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

clawson wrote:

I'd just use Dean's ringbuffer from LUFA. One .h file and that's it

 

Had to track that down, but yes, his implementation is really elegant.  May just switch to it.

 

But it does the thing I was wondering about in the way Paul suggested.

 

Thanks all!