snprintf() and double

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

Hello!

 

I've been trying to get a function to work for awhile and it's driving me nuts. I know there's an alternative in dtostrf(...) but now I'm just wondering what exactly is the problem even if I might not use it for my project (motion tracker/display using MPU-6050 data to visualize on a small SPI connected OLED) at this rate.

 

void XBeeSendInt(int16_t number)
{
	char* str = ConvertIntToStr(number);
	while(*str)
	{
		XBeeSendChar(*str++);
	}
	free(str);
}
void XBeeSendDouble(double dbl)
{
	char* str = ConvertDblToStr(dbl);
	while(*str)
		{
			XBeeSendChar(*str++);
		}
	free(str);
}
char* ConvertIntToStr(int16_t number)
{
	int length = snprintf( NULL, 0, "%d", number );
	char* str = malloc( length + 1 );
	snprintf( str, length + 1, "%d", number );
	return str;
}
char* ConvertDblToStr(double dbl)
{
	int length = snprintf( NULL, 0, "%f", dbl );
	char* str = malloc( length + 1 );
	snprintf( str, length + 1, "%f", dbl );
	return str;
}

So the int code works pretty well (I have it outputting to putty via uart) though I haven't really tried a whole bunch of values, but it does send at least. The character array that gets made for the converted int is a little bit past the starting address for the SRAM (0x002056).

 

However, the string for the double gets put at 0x005985 and all stored values in that section of memory are FF. It doesn't appear to be working at all. I've tried %f and %lf and I've made sure to check the linker (below) is correct to allow full floating point for vprintf().

-Wl,-Map="$(OutputFileName).map" -Wl,-u,vfprintf -Wl,--start-group -Wl,-lm -Wl,-lprintf_flt  -Wl,--end-group -Wl,--gc-sections 
-mmcu=atxmega128a1u -B "C:\Program Files (x86)\Atmel\Studio\7.0\Packs\atmel\XMEGAA_DFP\1.1.68\gcc\dev\atxmega128a1u" 

I'm not sure what the problem is at all at this point and could really use some help. Thank you!

This topic has a solution.
Last Edited: Thu. Nov 14, 2019 - 07:50 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

 

This looks like the kind of thing it should be possible to test in simulation. If I build:

#include <stdio.h>
#include <stdlib.h>
#include <avr/io.h>

char* ConvertIntToStr(int16_t number)
{
	int length = snprintf( NULL, 0, "%d", number );
	char* str = malloc( length + 1 );
	snprintf( str, length + 1, "%d", number );
	return str;
}
char* ConvertDblToStr(double dbl)
{
	int length = snprintf( NULL, 0, "%f", dbl );
	char* str = malloc( length + 1 );
	snprintf( str, length + 1, "%f", dbl );
	return str;
}

int main(void)
{
	char * str = ConvertDblToStr(3.141592);
	while (*str) {
		PORTB = *str++;
	}
}

As you can see this appears to work just fine:

 

BTW forgot to say that in avr-gcc sizeof(float)==sizeof(double)==4, they are all 32bit so you may be deceiving yourself if you think there is really "double" support - they are all just float sized. (so about 7.5 decimal digits).

Last Edited: Tue. Nov 12, 2019 - 12:34 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Well that's frustrating, I'll try simulating that later today to see if I get the same results as you. I've been running the code on an 128a1u all this time and it actually breaks putty when I try to send out a test double, but the int and text shows up fine.

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

clawson wrote:

 

This looks like the kind of thing it should be possible to test in simulation. If I build:...

 

I tested in simulation and was able to get it to work no problem, but for some reason it just won't work properly with my existing project/code. Maybe you can spot something I'm missing.

#ifndef F_CPU
#define F_CPU 32000000
#endif

#include <avr/io.h>
#include <avr/interrupt.h>
#include <avr/pgmspace.h>
#include <stdio.h>
#include <stdlib.h>
#include <util/delay.h>
#include <string.h>
#include <math.h>

void xMega32MHZ();
void InterruptInit();
void XBeeSerialInit();
void XBeeSendChar(char);
void XBeeSendString(char*);
void XBeeSendInt(int16_t);
void XBeeSendDouble(double);
char XBeeReceiveChar();
char* ConvertIntToStr(int16_t);
char* ConvertDblToStr(double);

int main(void)
{
	xMega32MHZ();
	InterruptInit();
	XBeeSerialInit();
	
	_delay_ms(100);
	XBeeSendString("Test");
	XBeeSendInt(6969);
	XBeeSendDouble(3.141592);
	
	_delay_ms(100);

	while (1)
	{
	}
}
void XBeeSerialInit()
{
	PORTF.DIRCLR = PIN2_bm;
	PORTF.DIRSET = PIN3_bm;
	PORTF.OUTSET = PIN3_bm;

	//BSEL = 983, BSCALE = -7 for 230400 Baud Rate
	USARTF0_BAUDCTRLB = 0x93;
	USARTF0_BAUDCTRLA = 0xD7;

	USARTF0_CTRLB = USART_TXEN_bm | USART_RXEN_bm;
	USARTF0_CTRLC = USART_CMODE_ASYNCHRONOUS_gc | USART_PMODE_DISABLED_gc | USART_CHSIZE_8BIT_gc;
}
void XBeeSendChar(char c)
{
	while(!(USARTF0_STATUS & USART_DREIF_bm)); //Wait until DATA buffer is empty
	USARTF0_DATA = c;
}
void XBeeSendString(char* str)
{
	while(*str)
	{
		XBeeSendChar(*str++);
	}
}
void XBeeSendInt(int16_t number)
{
	char* str = ConvertIntToStr(number);
	while(*str)
	{
		XBeeSendChar(*str++);
	}
	free(str);
}
void XBeeSendDouble(double dbl)
{
	char* str = ConvertDblToStr(dbl);
	while(*str)
		{
			XBeeSendChar(*str++);
		}
	free(str);
}
char* ConvertIntToStr(int16_t number)
{
	int length = snprintf( NULL, 0, "%d", number );
	char* str = malloc( length + 1 );
	snprintf( str, length + 1, "%d", number );
	return str;
}
char* ConvertDblToStr(double dbl)
{
	int length = snprintf( NULL, 0, "%lf", dbl );
	char* str = malloc( length + 1 );
	snprintf( str, length + 1, "%lf", dbl );
	return str;
}
char XBeeReceiveChar()
{
	while(!(USARTF0.STATUS & USART_RXCIF_bm)); //  wait for new character
	return USARTF0.DATA;
}
void xMega32MHZ()
{
    CCP = CCP_IOREG_gc;						// disable register security for oscillator update
    OSC.CTRL = OSC_RC32MEN_bm;				// enable 32MHz oscillator
    while(!(OSC.STATUS & OSC_RC32MRDY_bm)); // wait for oscillator to be ready
    CCP = CCP_IOREG_gc;						// disable register security for clock update
    CLK.CTRL = CLK_SCLKSEL_RC32M_gc;		// switch to 32MHz clock
}
void InterruptInit()
{
	PMIC.CTRL |=  PMIC_LOLVLEN_bm;
	sei();
}

It's still not properly converting the double value when I try to run it on my 128A1U.

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

There should be absolutely zero reasons why converting from a double would not work on a 128A1U. Something is being overlooked.

 

How are you creating this double that you test with? Putty should not care. In only receives ASCII characters. What is the string that results from the  conversion?

 

Jim

Jim Wagner Oregon Research Electronics, Consulting Div. Tangent, OR, USA http://www.orelectronics.net

Last Edited: Tue. Nov 12, 2019 - 08:56 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Presumably you are saying that the terminal shows "Test6969" then just stops?

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

i) are the linker arguemts for float suppot definitely correct ? (I don't know if they are or not)

ii) what is it returning for length?

iii) does using a statically allocated char buf rather than malloc make any difference?

 

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

is %lf valid with avr-libc?

 

https://www.microchip.com/webdoc/AVRLibcReferenceManual/group__avr__stdio_1gaa3b98c0d17b35642c0f3e4649092b9f1.html

 

I suspect that the conversion specifier system does not know how to use floats in the place of doubles.

 

update: a key word was missing

Last Edited: Tue. Nov 12, 2019 - 09:11 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hmmm,

 

char* ConvertDblToStr(double dbl)
{
	int length = snprintf( NULL, 0, "%lf", dbl );
	char* str = malloc( length + 1 );
	snprintf( str, length + 1, "%lf", dbl );
	return str;
}

Everything I have read says that malloc() is bad news on a memory-constrained micro.

 

Jim

Jim Wagner Oregon Research Electronics, Consulting Div. Tangent, OR, USA http://www.orelectronics.net

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

The way he's  using malloc should be OK as there's only one thing allocated and then freed after use each time. But it does kind of raise the question of why not simply use a global "scatchpad" buffer allocated by the linker.

 

BTW #1 and then my simulator quest that mimicked it just used %f, not sure where %lf sprang from?

Last Edited: Tue. Nov 12, 2019 - 09:09 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

ron_sutherland wrote:

is %lf valid with avr-libc?

It should be if it follows C99 onwards.

For printf conversion, the l length modifier  "has no effect on a following a, A, e, E, f, F, g, or G conversion specifier."

So %f and %lf are both allowed and mean same thing for printf  (Any float argument to printf is promoted (default promotions) to double, so there is no printf conversion specifier for float. This is not the csase for scanf).

The fact that on avr a double is really just a float, I don't think that should cause any problem.

 

 

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

ka7ehk wrote:

There should be absolutely zero reasons why converting from a double would not work on a 128A1U. Something is being overlooked.

 

How are you creating this double that you test with? Putty should not care. In only receives ASCII characters. What is the string that results from the  conversion?

 

Jim

Here's what putty puts out before it crashes:

 

clawson wrote:

Presumably you are saying that the terminal shows "Test6969" then just stops?

 

Yes, see the picture.

 

MrKendo wrote:

i) are the linker arguemts for float suppot definitely correct ? (I don't know if they are or not)

ii) what is it returning for length?

iii) does using a statically allocated char buf rather than malloc make any difference?

 

 

1) Pretty sure they are. Checked vprintf() option in general toolchain settings and added libprintf_flt to linked libraries. libm was already there.
2) The double passes through (I changed it to 3.141592), and length reports as 8 at the "char* str = malloc( length + 1 );" line when a breakpoint is there. Then the str* is at some 0x005XXX value instead of near the beginning of the stack at 0x002000 like the int version of the code does.

3) 

char* ConvertDblToStr(double dbl)
{
    //int length = snprintf( NULL, 0, "%lf", dbl );
    //char* str = malloc( length + 1 );
    static char* str[8];
    snprintf( str, 9, "%lf", dbl );
    return str;
}

This works (not sure why exactly), but not sure what best practice would be. I doubt the malloc method is best practice either so if someone knows what that is I will gladly change the code for that.
Also gives some errors:

Message        expected 'char *' but argument is of type 'char **'    Prototyping    stdio.h    687 (extern int    snprintf(char *__s, size_t __n, const char *__fmt, ...);)
Warning        passing argument 1 of 'snprintf' from incompatible pointer type [-Wincompatible-pointer-types] (second last line of function)
Warning        return from incompatible pointer type [-Wincompatible-pointer-types] (last line of function)

 

ron_sutherland wrote:

is %lf valid with avr-libc?

 

https://www.microchip.com/webdoc/AVRLibcReferenceManual/group__avr__stdio_1gaa3b98c0d17b35642c0f3e4649092b9f1.html

 

I suspect that the conversion specifier system does not know how to use floats in the place of doubles.

 

update: a key word was missing

Because AVR doesn't have real doubles I don't think it actually matters. %lf or %f will be treated the same.

 

ka7ehk wrote:

Hmmm,

 

char* ConvertDblToStr(double dbl)
{
	int length = snprintf( NULL, 0, "%lf", dbl );
	char* str = malloc( length + 1 );
	snprintf( str, length + 1, "%lf", dbl );
	return str;
}

Everything I have read says that malloc() is bad news on a memory-constrained micro.

 

Jim

 

Could you explain this a bit more?

 

clawson wrote:

The way he's  using malloc should be OK as there's only one thing allocated and then freed after use each time. But it does kind of raise the question of why not simply use a global "scatchpad" buffer allocated by the linker.

 

BTW #1 and then my simulator quest that mimicked it just used %f, not sure where %lf sprang from?

Could you explain this more? I would certainly want to do whatever is best practice.

 

The raw MPU data is int16 x,y,z for acceleration and angular velocity (x6 int16) that gets converted to double (x6 double) when divided by the respective gains for acceleration and angular velocity.

I need to be able to effectively send this information wirelessly via serial to be processed and draw the general path of motion of the device. This conversion is merely for displaying information on the OLED screen and I know it can be easily sent via serial and displayed while I'm debugging.

 

Last Edited: Wed. Nov 13, 2019 - 02:25 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I needed to add this for floating point support in snprintf for a recent project.  It was an Arduino project using a SAMD21 (arm) so not sure if it required in your case.

// Needed for floating point support in snprintf
asm(".global _printf_float");

 

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

Astray wrote:

ka7ehk wrote:

Everything I have read says that malloc() is bad news on a memory-constrained micro.

 

Jim

 

Could you explain this a bit more?

 

 

 

Heap fragmentation, memory leaks, and heap exhaustion are a few of the problems you can run into when using dynamic memory allocation in an embedded system if you aren't careful. In many cases, including yours, you can avoid dynamic memory allocation if you spend a little more time at the drawing board.

 

In your application you should be able to determine a reasonable upper bound for the length of the strings generated by your conversions at compile time. You could then modify the signatures of ConvertIntToStr() and ConvertDblToStr() to make the caller responsible for providing the buffer the conversion is stored in, but at that point you might as well just have the callers call snprintf() directly. The buffers could be allocated on the stack by the callers of snprintf(), or they could be a global "scrathpad" buffer as clawson suggested. If a global buffer is used there is no need to pass the storage to the conversion functions since they could access it directly, but you would need to be extremely careful about the lifetime of anything placed in the buffer.

 

C99 added support for stack allocated variable-length arrays which can be used as an alternative to dynamic memory allocation. C11 made support for stack allocated variable-length arrays optional.

 

Some notes on your code:

  • Dynamically allocating memory and then using the result of the allocation without ensuring it succeeded is an extremely bad practice.
  • When using the printf() family of functions with fixed-width integer types, you should be using the format specifier macros provided in inttypes.h, not the format specifiers for built-in types. Since int16_t happens to be int with avr-gcc (so long as you aren't using the option that makes int 8-bit), you happen to not encounter any problems. If GCC's -Wformat warning is enabled (enabled by -Wall), the compiler will check for format string errors.

 

github.com/apcountryman/build-avr-gcc: a script for building avr-gcc

github.com/apcountryman/toolchain-avr-gcc: a CMake toolchain for cross compiling for the Atmel AVR family of microcontrollers

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

Astray wrote:

Here's what putty puts out before it crashes:

 

The question mark in the output where you expected to see pi is likely due to avr-libc's default (minimal) vfprintf() implementation not supporting floating point. This is explained in the notes section of vfprintf()'s documentation. Instructions for enabling floating point support are also provided in vfprintf()'s documentation.

 

 Edit:

MrKendo mentioned the need to enable floating point support earlier in the thread. ron_sutherland also linked to the vfprintf() documentation.

github.com/apcountryman/build-avr-gcc: a script for building avr-gcc

github.com/apcountryman/toolchain-avr-gcc: a CMake toolchain for cross compiling for the Atmel AVR family of microcontrollers

Last Edited: Wed. Nov 13, 2019 - 03:57 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Could an ARM M0 use the avr-libc shrink style of printf?

 

https://packages.debian.org/buster/libnewlib-nano-arm-none-eabi

 

That is interesting.

 

https://keithp.com/newlib-nano/

 

I found that while fishing for clues why %f might work, but %lf might not. Now back on subject, I know %f works (epccs is my github)

 

https://github.com/epccs/RPUno/blob/master/Adc/analog.c#L98

 

but I did have to turn on floats printing (forgot about that) with

 

https://github.com/epccs/RPUno/blob/master/Adc/Makefile#L53

 

I am not set up at the moment to try %lf.

Last Edited: Wed. Nov 13, 2019 - 04:38 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

apcountryman wrote:

Astray wrote:

ka7ehk wrote:

Everything I have read says that malloc() is bad news on a memory-constrained micro.

 

Jim

 

Could you explain this a bit more?

 

 

 

Heap fragmentation, memory leaks, and heap exhaustion are a few of the problems you can run into when using dynamic memory allocation in an embedded system if you aren't careful. In many cases, including yours, you can avoid dynamic memory allocation if you spend a little more time at the drawing board.

 

In your application you should be able to determine a reasonable upper bound for the length of the strings generated by your conversions at compile time. You could then modify the signatures of ConvertIntToStr() and ConvertDblToStr() to make the caller responsible for providing the buffer the conversion is stored in, but at that point you might as well just have the callers call snprintf() directly. The buffers could be allocated on the stack by the callers of snprintf(), or they could be a global "scrathpad" buffer as clawson suggested. If a global buffer is used there is no need to pass the storage to the conversion functions since they could access it directly, but you would need to be extremely careful about the lifetime of anything placed in the buffer.

 

C99 added support for stack allocated variable-length arrays which can be used as an alternative to dynamic memory allocation. C11 made support for stack allocated variable-length arrays optional.

 

Some notes on your code:

  • Dynamically allocating memory and then using the result of the allocation without ensuring it succeeded is an extremely bad practice.
  • When using the printf() family of functions with fixed-width integer types, you should be using the format specifier macros provided in inttypes.h, not the format specifiers for built-in types. Since int16_t happens to be int with avr-gcc (so long as you aren't using the option that makes int 8-bit), you happen to not encounter any problems. If GCC's -Wformat warning is enabled (enabled by -Wall), the compiler will check for format string errors.

 

 

I think I understand some of the ideas you're trying to get across, but if you could provide an example code snippet for me to study that would be incredibly helpful as I don't feel I'm completely grasping everything well enough to know how to implement your recommendations.

 

apcountryman wrote:

 

The question mark in the output where you expected to see pi is likely due to avr-libc's default (minimal) vfprintf() implementation not supporting floating point. This is explained in the notes section of vfprintf()'s documentation. Instructions for enabling floating point support are also provided in vfprintf()'s documentation.

 

 Edit:

MrKendo mentioned the need to enable floating point support earlier in the thread. ron_sutherland also linked to the vfprintf() documentation.

 

Please believe me when I say floating point support is properly enabled. There is some weird memory issue occurring as changing the code to 

char* ConvertDblToStr(double dbl)
{
    //int length = snprintf( NULL, 0, "%lf", dbl );
    //char* str = malloc( length + 1 );
    static char* str[8];
    snprintf( str, 9, "%lf", dbl );
    return str;
}

works perfectly fine. Also works if I use %f instead of %lf.

Last Edited: Wed. Nov 13, 2019 - 06:27 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Astray wrote:

    static char* str[8];
    snprintf( str, 9, "%lf", dbl );
    return str;

You are defining an array of ptr to char. You want an array of char. And remember to allow room for null termination

 

     static char str[9];
     snprintf( str, 9, "%f", dbl );
     return str;

This is OK for a quick test as a substitute for your original malloc version, but I wouldn't  do it this way normally. (It was already mentioned in an earlier reply by apcountryman,  normally you would let the caller be responsible for the storage and pass a pointer to the buffer into the function, that's if you even  need a function at all).

 

 

 

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


MrKendo wrote:
  normally you would let the caller be responsible for the storage and pass a pointer to the buffer into the function
As both itoa() and sprintf() do in fact.

 

As noted the output "Test6969?" means it is working. By default AVR-LibC links with a default copy of vfprintf(). It has alternates for the function in libprintf_fmin.a (a minimal version) and libprintf_flt.a (the float supporting version). Unless you use libprintf_flt.a then %f output is just "?".

 

The curious thing here though is that you said:

-Wl,-Map="$(OutputFileName).map" -Wl,-u,vfprintf -Wl,--start-group -Wl,-lm -Wl,-lprintf_flt  -Wl,--end-group -Wl,--gc-sections 
-mmcu=atxmega128a1u -B "C:\Program Files (x86)\Atmel\Studio\7.0\Packs\atmel\XMEGAA_DFP\1.1.68\gcc\dev\atxmega128a1u" 

which *should* be working. The -u,vfprintf says "pretend you haven't seen the default copy of vfprintf in the library". Then the -lprintf_flt means "link with libprintf_flta.a". So it *should* be replacing the default implementation of vfprintf() with the one that can do %f

 

So I would look at your build again. When I did this in the simulator all I did was:

 

 

That provides the -Wl,-lprintf_flt and also:

 

 

and that tells it to forget the default implementation so the one in libprintf_flt.a can be found. The results in:

		Building target: test.elf
		Invoking: AVR8/GNU Linker : 5.4.0
		"C:\Program Files (x86)\Atmel\Studio\7.0\toolchain\avr8\avr8-gnu-toolchain\bin\avr-g++.exe" -o test.elf  main.o   -Wl,-Map="test.map" -Wl,-u,vfprintf -Wl,--start-group 
                -Wl,-lm -Wl,-lprintf_flt  -Wl,--end-group -Wl,--gc-sections -mrelax -mmcu=atmega2561 -B "C:\Program Files (x86)\Atmel\Studio\7.0\Packs\atmel\ATmega_DFP\1.3.300\gcc\dev\atmega2561" 
                -Wl,-print-gc-sections -Wl,-section-start=.fram=0x10000000  
		Finished building target: test.elf

which is pretty much what you showed.

 

So I wonder how this is being defeated in your actual application? Have a look at the .map file and find out where vfprintf() is. For mine I see:

Archive member included to satisfy reference by file (symbol)

c:/program files (x86)/atmel/studio/7.0/toolchain/avr8/avr8-gnu-toolchain/bin/../lib/gcc/avr/5.4.0/../../../../avr/lib/avr6\libprintf_flt.a(vfprintf_flt.o)
                              (vfprintf)

which shows that vfprintf() is being delivered by the vfprintf_flt.o member of libprintf_flt.a

 

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

It seems like the fact that this function works if I write it like this is being overlooked:

char* ConvertDblToStr(double dbl)
{
	//int length = snprintf( NULL, 0, "%f", dbl );
	//char* str = malloc( length + 1 );
	static char str[9];
	snprintf( str, 9, "%f", dbl );
	return str;
}

 but if I try to use this function it doesn't work (some really weird memory shenanigans are happening with the pointer str not being put on the stack 0x002000 but at some random memory address 0x005XXX):
 

char* ConvertDblToStr(double dbl)
{
	int length = snprintf( NULL, 0, "%lf", dbl );
	char* str = malloc( length + 1 );
	snprintf( str, length + 1, "%lf", dbl );
	return str;
}

The linker is not the problem, snprintf does work if there's an empty string that already exists.

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

I was not aware of the "zero length" option.   But Googling found https://www.oreilly.com/library/view/c-in-a/0596006977/re210.html

 

Which says:

To obtain the length of the output string without generating it, you can set n equal to zero; in this case, sprintf() writes nothing to dest, which may be a null pointer.

I would be very suspicious.  Does your library implement this ?   i.e. I would check the return value.   But probably pass a genuine buffer instead of NULL.   Check whether the library is writing to this buffer.

 

I tend to use a statically allocated buffer.    This will always work with any Compiler on any platform.

 

And I certainly would not introduce memory leaks that you are deliberately putting into #20

 

David.

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

Or just:

char* str = malloc( 20 );

and see if that works. (I predict it will as I'm guessing that David is right and the limited version of snprintf() in avr-libc may not have this NULL/Len=0 option.

 

EDIT: yup, this is the AVR implementation of snprintf():

 

http://svn.savannah.gnu.org/viewvc/avr-libc/trunk/avr-libc/libc/stdio/snprintf.c?revision=1944&view=markup

 

I see nothing there to handle the NULL/Len=0 case - it's just assuming 's' is a valid pointer and n is a valid length.

Last Edited: Wed. Nov 13, 2019 - 12:43 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

clawson wrote:

I see nothing there to handle the NULL/Len=0 case - it's just assuming 's' is a valid pointer and n is a valid length.

 

But a part of the comments in that function does suggest that 0 is a valid value:

So we can write a negative value into f.size in the case of n was 0.

And it is also stated that f.size may be -1:

f.size = n - 1;   /* -1,0,...32767 */

The real magic must be happening in vfprintf.

 

Last Edited: Wed. Nov 13, 2019 - 03:11 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

vfprintf source is here:

 

http://svn.savannah.gnu.org/viewvc/avr-libc/trunk/avr-libc/libc/stdio/vfprintf.c?revision=2191&view=markup

 

There are two implementations of the function. The libprintf_flt.a one has the PRINTF_FLT handling from about line 370 onwards. 

 

Remember that on entry to this the only indication not to output is stream->size = -1. I personally cannot see anything there handling this as a special case. 

 

The only use I see made of the .stream field in the FILE structure is back in snprintf() itself where it does:

        if (f.size >= 0)
63	                s[f.len < f.size ? f.len : f.size] = 0;

IOW it only writes a terminating zero is size is zero or postitive. But this seems to be the only use of the -1 that has been put there.

 

Actually I don't see anything in the implementation that is policing the N in sprintf(buff, N, ...) at all ?? The implication would seem to be that the length field is actuall superfluous in snprintf() and you might as well just use sprintf(). 

 

Off to try a simulator experiment to try and overflow a buffer...

 

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

clawson wrote:
Off to try a simulator experiment to try and overflow a buffer...
OK, so it is policed inside fputc(). Bang goes that theory ;-)

This reply has been marked as the solution. 
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Astray wrote:

void XBeeSendInt(int16_t number)
{
	char* str = ConvertIntToStr(number);
	while(*str)
	{
		XBeeSendChar(*str++);
	}
	free(str);
}

I just spotted.

str is incremented, so the str in free(str) is wrong!

So I expect the second call to malloc goes bad.

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

MrKendo wrote:

Astray wrote:

void XBeeSendInt(int16_t number)
{
	char* str = ConvertIntToStr(number);
	while(*str)
	{
		XBeeSendChar(*str++);
	}
	free(str);
}

I just spotted.

str is incremented, so the str in free(str) is wrong!

So I expect the second call to malloc goes bad.


This may be it because the null case for snprintf does appear to work for int! I'll do some more investigating later today.

Either way I need to rewrite this to be better code regardless. From what you guys are saying I should change the function call to DoubleToStr(double, char*) right?

Last Edited: Wed. Nov 13, 2019 - 07:53 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

As a test, if you change your existing SendInt and SendDbl functions to like below then I expect it to then work

 

void XBeeSendInt(int16_t number)
{
	char* str = ConvertIntToStr(number);
        char *p = str;
	while(*str)
	{
		XBeeSendChar(*str++);
	}
	free(p);
}

Ultimately I would get rid of malloc and just use a fixed size array (big enough for whatever the max will be).

 

There's not much benefit now having separate ConvertDbl function.

So may as well be more like

 

void XBeeSendDouble(double dbl)
{
    char str[20];
    snprintf(str, 20, "%f", dbl);

    /* EDIT doh! of course can't modify str like this!
    while(*str)
    {
        XBeeSendChar(*str++);
    }
    */

    char *p = str;
    while(*p)
    {
        XBeeSendChar(*p++);
    }

    /* EDIT or better still create a send string func and use that
    XbeeSendStr(str);
    */
}

 

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

MrKendo wrote:

Astray wrote:

void XBeeSendInt(int16_t number)
{
	char* str = ConvertIntToStr(number);
	while(*str)
	{
		XBeeSendChar(*str++);
	}
	free(str);
}

I just spotted.

str is incremented, so the str in free(str) is wrong!

So I expect the second call to malloc goes bad.

 

This is the problem, I switched the calls to the functions for int and double and the int function resulted in the same problem.

 

I wonder if there's a way to still use the snprintf null function to create the appropriate sized string without using malloc as I like that functionality. 

 

MrKendo wrote:

As a test, if you change your existing SendInt and SendDbl functions to like below then I expect it to then work

 

void XBeeSendInt(int16_t number)
{
	char* str = ConvertIntToStr(number);
        char *p = str;
	while(*str)
	{
		XBeeSendChar(*str++);
	}
	free(p);
}

Ultimately I would get rid of malloc and just use a fixed size array (big enough for whatever the max will be).

 

There's not much benefit now having separate ConvertDbl function.

So may as well be more like

 

void XBeeSendDouble(double dbl)
{
    char str[20];
    snprintf(str, 20, "%f", dbl);

    /* EDIT doh! of course can't modify str like this!
    while(*str)
    {
        XBeeSendChar(*str++);
    }
    */

    char *p = str;
    while(*p)
    {
        XBeeSendChar(*p++);
    }

    /* EDIT or better still create a send string func and use that
    XbeeSendStr(str);
    */
}

 

 

I changed the functions to handle parsing the strings and freeing the memory as you wrote and the code works now so we know for sure that was the issue.

 

I would like to keep the functions separate as the usage of DblToStr is not to send it via serial but for eventual display on my LCD.

 

Is there some recommended reading you could point me to in regards to passing arguments into functions while minimizing memory usage?

 

My understanding of memory handling in regards to C is definitely lacking it seems and I would like to minimize memory costs just to be safe for my application.

 

apcountryman wrote:

Some notes on your code:

  • Dynamically allocating memory and then using the result of the allocation without ensuring it succeeded is an extremely bad practice.
  • When using the printf() family of functions with fixed-width integer types, you should be using the format specifier macros provided in inttypes.h, not the format specifiers for built-in types. Since int16_t happens to be int with avr-gcc (so long as you aren't using the option that makes int 8-bit), you happen to not encounter any problems. If GCC's -Wformat warning is enabled (enabled by -Wall), the compiler will check for format string errors.

 

Hoping you could go into more detail about these statements as I didn't quite understand them, at least not enough to implement them. Examples are extremely helpful for understanding ideas personally.

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

Astray wrote:
I wonder if there's a way to still use the snprintf null function to create the appropriate sized string without using malloc as I like that functionality. 
C99 introduced variable length arrays so you can do things like:

void foo(int n) {
    char buff[n];
}

I just test built:

void foo(char * str) {
	int i;
	char buff[strlen(str)];
	for (i = 0; i < strlen(str); i++) {
		buff[i] = str[i];
	}
}

and it certainly seems to compile OK. Haven't tried using it yet though.

 

EDIT: with sufficient volatile applied that works just fine. One thing that surprised me is that with strlen() in the for() loop it really does thoe whole strlen() every time - I thought it would cache the result but it doesn't so best to force this manually:

void foo(char * str) {
	int i, len;
	volatile char buff[strlen(str)];
	len = strlen(str);
	for (i = 0; i < len; i++) {
		buff[i] = str[i];
	}
}

 

Last Edited: Thu. Nov 14, 2019 - 10:40 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

You can use VLA if you want, the difference from malloc being it will now be on the stack, so you have to use it within the same function.

Fir a 16 bit int, however, you know the max size is 5 digits, plus possibly a sign, plus null, so 7.

For float, it's not so straightforward if you use %f, where the precision (default 6 so equivalent to %.6f) represents the number of places after decimal point, says nothing about how many digits before the dec point.

So if your float can cover a wide range you might have to allow up to about 39 digits before dec point (for a number like 1e38).

It's much easier to predict if you use %g or %e.

With %g for example, the precision (again default 6 so equivalent to %.6g) represents the number of significant digits.

So the longest string you can get is one of

-1.23456e+nn

or

-0.000123456

either way max 12, so plus null = 13 max.

 

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

Astray wrote:

Hoping you could go into more detail about these statements as I didn't quite understand them, at least not enough to implement them. Examples are extremely helpful for understanding ideas personally.

 

malloc(), calloc(), and realloc() can fail to allocate memory. When they do fail, they return a null pointer which you must check for and handle. Dereferencing a null pointer results in undefined behavior.

  • In the case of writing to a dereferenced null pointer, you are overwriting whatever happens to be at the address a null pointer points to (likely address 0 but this can vary depending on the implementation) which will almost certainly lead to your running program exploding at some point in the future.
  • In the case of reading from a dereferenced null pointer, you may be reading memory that doesn't contain the type/value you expect which could lead to your running program exploding at some point in the future.

Writing to and reading from dangling pointers pose the same dangers. Calling free() on a pointer that has already been freed or on a pointer that was not returned by malloc(), calloc(), or realloc() can also cause your running program to explode at some point in the future.

 

Format specifiers for fixed width integer types are defined in inttypes.h. These should always be used when working with fixed width integer types since you do not know a fixed width type's underlying built in type. If you use the built in format specifiers for a type that ends up being promoted to int/unsigned int, this ends up not actually mattering. However, in cases where an assumed promotion does not occur (such as an implementation where int_fast8_t is actually long) you will get unexpected behavior. A compiler can catch these kinds of format string mismatches if the appropriate warnings are enabled.

 

The following example checks for memory allocation failures, and makes use of fixed width integer type format specifiers. The snprintf() failure checks are generally overkill (or at least better done with assert() or something with similar behavior) since failures are usually (always?) due to misuse, some of which can be caught by the compiler if the appropriate warnings are enabled.

char * convert_int16_t_to_string( int16_t number )
{
    int buffer_size = snprintf( NULL, 0, "%" PRId16, number ) + 1;
    if ( buffer_size <= 0 ) {
        return NULL;
    } // if

    char * string = malloc( buffer_size );
    if ( !string ) {
        return NULL;
    } // if

    if ( snprintf( string, buffer_size, "%" PRId16, number ) < 0 ) {
        free( string );

        return NULL;
    } // if

    return string;
}

 

github.com/apcountryman/build-avr-gcc: a script for building avr-gcc

github.com/apcountryman/toolchain-avr-gcc: a CMake toolchain for cross compiling for the Atmel AVR family of microcontrollers

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

avr-gcc is moving slowly towards double:

https://savannah.nongnu.org/bugs/?57071#comment9

avrfreaks does not support Opera. Profile inactive.

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

Georg-Johann, your contributions to this compiler are incredible. It's making this a really great tool. What a shame if it's deprecated at V10 and removed at V11!

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

So after reading through I decided to change my approach and used VLA. In order to do so it didn't make any sense to keep the Convert functions so I inlined them. Here is the result:

void XBeeSendInt(int16_t number)
{
    int16_t length = snprintf( NULL, 0, "%" PRId16, number );
    char temp[length];
    char* str = temp;
    snprintf( str, length + 1, "%" PRId16, number );
    while(*str)
    {
        XBeeSendChar(*str++);
    }
}
void XBeeSendDouble(double dbl)
{
    int16_t length = snprintf( NULL, 0, "%f", dbl );
    char temp[length];
    char* str = temp;
    snprintf( str, length + 1, "%f", dbl );
    while(*str)
        {
            XBeeSendChar(*str++);
        }
}

Let me know what you guys think.

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

Astray wrote:
Let me know what you guys think.
Still looks like over-engineering to me! I'd have gone for:

void XBeeSendInt(int16_t number)
{
    char temp[20]; // 20 should be more than enough in all cases!!
    char* str = temp;

    snprintf( str, 20, "%" PRId16, number );
    while(*str)
    {
        XBeeSendChar(*str++);
    }
}

or perhaps more generally:

char reusable_buffer[32];

void XBeeSendInt(int16_t number)
{
    char* str = reusable_buffer;
    snprintf( str, 32, "%" PRId16, number );
    while(*str)
    {
        XBeeSendChar(*str++);
    }
}

Then reuse the same global/shared buffer wherever you have the need for a temporary thing holding up to 32 bytes.

 

The advantage of the global is that it's allocate by the linker at build time so you'll know immediately if there's a RAM shortage. With the local it's going to depend on dynamic RAM/stack usage at the time.

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

I was always taught that globals were to be frowned upon in programming, but I guess that might not be the case with embedded solutions like an xmega. Is there a rule of thumb here that would be good to know for globals?

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

Data encapsulation should always be a goal in modular (maintainable!) software. But, yeah, in resource limited micros it's not unusual to have some kind of general "scratchpad" buffer that you share across several tasks. One could envisage some kind of lock/free mechanism around such a buffer if you wanted to protect it from dual use. (so user has to get a lock (mutex?)) first before they can use it) if you were really worried.

 

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

Astray wrote:
I was always taught that globals were to be frowned upon in programming

Indeed.

 

 but I guess that might not be the case with embedded solutions like an xmega. Is there a rule of thumb here that would be good to know for globals?

clawson explained the specific benefit of the global in this case:

clawson wrote:
The advantage of the global is that it's allocate by the linker at build time so you'll know immediately if there's a RAM shortage. With the local it's going to depend on dynamic RAM/stack usage at the time.

 

As always in Engineering, there's a compromise - you have to weigh the advantages against the disadvantages in the light of the specific system requirements & constraints.

 

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...