Question about memory allocation - AVR

Log in or register to post comments
Go To Last Post
6 posts / 0 new
Author
Message
Krupski
Krupski's picture
Level: Hangaround
Joined: Tue. Nov 17, 2015
Posts: 243 View posts
Location: NY, USA
#1
Posted by Krupski: Sun. Oct 3, 2021 - 11:26 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hi everyone.

 

I wrote a small library for the AVR family which enables easy use of STDIN, STDOUT and STDERR (you can look at the code HERE).

 

Basically, it accepts a device object (such as "Serial" or "LCD", etc..) and allows using printf, getc and other functions to write or read a character device).

 

The library currently also has a pushStream and popStream function (not shown in the old github upload) that allows saving the current device, setting a new device, then restoring the original device (like a stack push and pop). To clarify, it would be used like this:

 

void debug (void)
{
    STDIO.pushStream (LCD); // save current device, setup LCD
    printf ("Cursor pos: %d\n", curpos); // show cursor pos on LCD
    STDIO.popStream(); // restore std streams to previous
}

Currently, there is only one set of pointers in which to save the streams before setting new ones. This means that if more than one pushStream is called, only the last one is saved, the previous ones are lost.

 

I modified the code to provide multiple stream pointers, accessed with an index. Like this:

 

Before:

Print *_stream_ptr0_save; // save standard in pointer

.... // two more - for stdout and stderr

 

After:

Print *_stream_ptr0_save[16]; // save up to 16 stdin pointers

.... // two more - for stdout and stderr

 

This works, but I don't like it because saving less than 16 pointers is a waste of memory and trying to save more obviously fails.

 

I want to use the realloc function to dynamically create new "slots" as needed.

 

My questions are:

  • Can I use realloc to also decrease the number of slots (i.e. when someone does a popStream() the save slots should be deallocated and the ram freed.
  • When the last slot is popped and deallocated, can I consider the memory "free" (i.e. would not need to do a call to free(_pointer))?

 

I know that supposedly the malloc and realloc functions are (unreliable)? in avr-gcc. However, I have another function to do a readline (get typed text from a device... usually Serial) and this function uses realloc for EACH character typed. Of course, the readline buffer needs to be freed after each call, but it works reliably... have NEVER had an error or crash, so I personally trust avr-gcc memory allocation.

 

So, do you think that the final realloc will have the effect of freeing the pointer allocation?

 

Thanks!

Gentlemen may prefer Blondes, but Real Men prefer Redheads!

Tags:

Tools, Compilers and General Programming
Top
apcountryman
apcountryman's picture
Level: Hangaround
Joined: Sun. Sep 11, 2016
Posts: 154 View posts
Location: WA, USA
Posted by apcountryman: Sun. Oct 3, 2021 - 11:54 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

The problems with using dynamic memory allocation are not specific to AVR microcontrollers or avr-gcc. They apply to all microcontrollers and cross compilers, and have nothing to do with the "reliability" of an implementation (though an buggy implementation would make things even worse). If you don't know why dynamic memory allocation is typically avoided in embedded applications, you probably shouldn't be using it. Here is a partial list of what can go wrong:

  • Potentially non-deterministic time required to allocate
  • Heap fragmentation leading to a memory allocation failure
  • Memory leak leading to a memory allocation failure
  • Not checking for memory allocation failures leading to a NULL pointer dereference
  • Double free resulting in undefined behavior

 

As far as your questions are concerned:

  • realloc() can be used to shrink a dynamically allocated buffer.
  • Every call to malloc() must have a corresponding call to free(). If you don't do this, you will have a memory leak.

 

Calling realloc() for every new character is extremely inefficient. You should take a look at what std::vector implementations do when existing capacity is full.

 

Did you consider using a circular buffer or some other technique that doesn't require dynamic memory allocation?

build-avr-gcc: avr-gcc build script

toolchain-avr-gcc: CMake toolchain for cross compiling for the AVR family of microcontrollers

avr-libcpp: C++ standard library partial implementation (C++17 only) for use with avr-gcc/avr-libc

picolibrary: C++ microcontroller driver/utility library targeted for use with resource constrained microcontrollers

picolibrary-microchip-megaavr: picolibrary HIL for megaAVR microcontrollers

picolibrary-microchip-megaavr0: picolibrary HIL for megaAVR 0-series microcontrollers

Top
apcountryman
apcountryman's picture
Level: Hangaround
Joined: Sun. Sep 11, 2016
Posts: 154 View posts
Location: WA, USA
Posted by apcountryman: Mon. Oct 4, 2021 - 12:16 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Also, what is the use case for this library? Constantly changing the device that stdout, stderr, and/or stdin use seems like it would just create confusion for anyone trying to understand the source code. avr-libc has the I/O functions that accept a FILE pointer so user functions that need to do I/O can simply have a FILE pointer parameter that tells them what to use. The user could also just change what avr-libc's stdout, stderr, and stdin point to as follows though this would cause the same confusion mentioned earlier.

static FILE lcd; // assume this is properly initialized somewhere else

void foo( void )
{
    FILE * temp = stdout;
    stdout = &lcd;
    puts( "foo" );
    stdout = temp;
}

Edit:

 

From looking at the code on GitHub, it looks like this is meant to be used with Arduino's I/O libraries. Again, I would question why the "standard" streams are being changed in the middle of program execution. Any function that needs to perform stream based I/O can simply take a reference/pointer to the appropriate Arduino I/O base class to control what it writes output to / reads input from.

build-avr-gcc: avr-gcc build script

toolchain-avr-gcc: CMake toolchain for cross compiling for the AVR family of microcontrollers

avr-libcpp: C++ standard library partial implementation (C++17 only) for use with avr-gcc/avr-libc

picolibrary: C++ microcontroller driver/utility library targeted for use with resource constrained microcontrollers

picolibrary-microchip-megaavr: picolibrary HIL for megaAVR microcontrollers

picolibrary-microchip-megaavr0: picolibrary HIL for megaAVR 0-series microcontrollers

Last Edited: Mon. Oct 4, 2021 - 12:33 AM
Top
Bronze Markjs
js's picture
Level: Moderator
Joined: Wed. Mar 28, 2001
Posts: 31343 View posts
Location: Sydney, Australia (Gum trees, Koalas and Kangaroos, No Edelweis, but plenty of Corona beer)
Posted by js: Mon. Oct 4, 2021 - 01:27 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Not knowing any fancy stuff I have always used fprintf or fprintf_P wink Some examples

 

fprintf_P (lcd,PSTR (" *OVER*"));       //Print to LCD

fprintf(com0, "Printing on com 0 with fprintf\r\n");    //Print to COM0

fprintf(com1, "Printing on com 1 with fprintf\r\n");    //Print to COM1

fprintf_P(display,PSTR("\rReady"));     //Print to a 5x7 matrix display

 

John Samperi

Ampertronics Pty. Ltd.

https://www.ampertronics.com.au

* Electronic Design * Custom Products * Contract Assembly

Top
curtvm
curtvm's picture
Level: Raving Lunatic
Joined: Wed. Sep 21, 2005
Posts: 3711 View posts
Posted by curtvm: Mon. Oct 4, 2021 - 06:16 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I would assume this is wanted to get to printf style printing instead of arduino style. The stdin/stdout are unneeded obstacles, and you can skip them and bypass printf and make use of fprintf/vfprintf instead.

 

The stdio family output for avr is blocking anyway, so just put the FILE struct on the stack. For output, you only need to set two struct member values. You can either make a simple function template that gives you a similar style like fprintf (no modifications to arduino code)-

    fprint( Serial, "123%d\r\n", 456 );

or better just give the Print class the ability to use printf with a little code added to the Print class-

    Serial.printf( "123%d\r\n", 456 );

 

https://godbolt.org/z/ahh85hfzs

 

Now every Print instance can have access to printf style, and there is no need to deal with any FILE business outside the single temporary use on the stack.

 

 

edit-

You can also deal with format strings in flash using what arduino has in place-

 

https://godbolt.org/z/YsnGnxP74

 

This example also separates out the static put functions where the object instance pointer is stored in the FILE udata member. Also a single function template was created with a std::is_same helper to route to either fprintf or fprintf_P, but can also make separate function templates for each type.

 

I don't use arduino, so the examples are not tested but should work as intended.

Last Edited: Mon. Oct 4, 2021 - 05:39 PM
Top
Gold Markclawson
clawson's picture
Level: Moderator
Joined: Mon. Jul 18, 2005
Posts: 105099 View posts
Location: (using avr-gcc in) Finchingfield, Essex, England
Posted by clawson: Mon. Oct 4, 2021 - 08:39 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

A few things:

 

1) stdin, stdout, stderr are simply pointers - in AVR that means 16 bits - would it really be that onerous to pre-allocate a fixed [16] of them? - for stdout that would be 32 bytes 

 

2) you could have the c'tor for your STDIO class (don't like the all upper case name by the way!) passed in the 16 or whatever so it dimensions the array at construction

 

3) it's new not malloc usually in C++. So something like:

STDIO::STDIO(int nStreams) {
    mPrntSave = new PrintPtr[nStreams];
    ...
}

~STDIO::STDIO() {
    delete mPrntSave;

 

 

Top