sscanf failure that does not make sense

[Scott.Bonomi]

As part of some diagnostic testing, I am trying to parse a from a serial port command into 5 fields.
The command is:
count = sscanf( InString, ShortForm, &R_W, &addr, &cnt, &bits, &value);
If I get count equal to 5, it should have 5 converted values, I thought.

This is most of the section of code
===============================================================
const char * ShortForm = "%c, 0x%x, %d, %d, %d" ;
int16_t addr=0, cnt=0, bits=0, value=9090;
char R_W;
size_t len = strlen(InString);
ESP_LOGI(TAG, " Parse Starting on,%s,", InString );
// if more than a minimum string
if ( len > 14 )
{
count = sscanf( InString, ShortForm, &R_W, &addr, &cnt, &bits, &value);
if (count < 5 )
ESP_LOGW(TAG, " Parse Error = %d, from,%s", count, InString );
else
{
ESP_LOGI(TAG, " Parse Good = %d, from,%s", count, InString );
ESP_LOGI(TAG, " Dir =%c(0x%04x), addr = %03x, cnt = %d, bits = %d, value = %d",R_W,R_W,addr,cnt,bits,value );
// == change added here
if ( cnt < 9 ) // single byte write
{
switch ( R_W ) // was toupper(R_W)
{
case 'W':
Ret_Val = Adi_Spi_Send( addr, (uint8_t)( value&0xFF) );
ESP_LOGE(TAG, " Parse Complete WRITE got %d",Ret_Val );
break;
case 'R':
Ret_Val = Adi_Spi_Read( addr, (uint8_t*) &value );// Explosion ??
ESP_LOGE(TAG, " Parse Complete READ got %d, %d", value, Ret_Val );
break;
default:
ESP_LOGE(TAG, " Parse Fail on R/W Bit for first non-white byte of %s", InString );
break;
}// switch

} // end if short cnt
// yes there is more code below here to complete the function.
===============================================================
===============================================================
sending this "R, 0x333, 001, 008, 015," No quotes and a CR.
===============================================================
===============================================================
and for a result I get
I (12206) Parser2SPI: Parse Starting on,R, 0x333, 001, 008, 015,, //commas around working string start and finish
I (12206) Parser2SPI: Parse Good = 5, from,R, 0x333, 001, 008, 015, //says it got 5 converted
I (12216) Parser2SPI: Dir =(0x0000), addr = 333, cnt = 1, bits = 8, value = 15 // First field did not convert the R
// writing a \0 between the = and (
E (12226) Parser2SPI: Parse Fail on R/W Bit for first non-white byte of R, 0x333, 001, 008, 015,
W (12236) Parser2SPI: Parse Finishing on,R, 0x333, 001, 008, 015,,:: 5
===============================================================
===============================================================
If I get 5 from the sscanf step, that should mean I got 5 conversions,
const char * ShortForm = "%c, 0x%x, %d, %d, %d" ; // R/W, 0xAddr, cnt, 8/16, Val
except the first conversion seems to be a failure.
===============================================================

NOW FOR THE THE REALLY SCARY PART = = = = > I added the code below and it works
count = sscanf( InString, "%c ", &R_W );// read it again
ESP_LOGI(TAG, " ReRead of Dir =%c(0x%04x), count is now %d\n",R_W,R_W, count );

gets ( in a different run.
I (235246) Parser2SPI: ReRead of Dir =W(0x0057), count is now 1

I tried white space before the %c, a good practice from the dark ages.
I tried the input string with and without white space.
I tried changing to %hx and %hd read formats for 16bit values. That did not change the results.

I expect library calls to work, and work the same way every time.

[nvannote]

Code: Select all

const char * ShortForm = "%c, 0x%x, %d, %d, %d" ; 

The following should work fine. Tested with clang, gcc and xtensa-gcc.

Code: Select all

const char * ShortForm = "%c, 0x%hX, %hd, %hd, %hd";

Your format string is specifying integers (32-bit on the ESP32), when your pointers are to shorts (16-bit). sscanf will do what it is told and treat the pointers as int(s), hence the corruption.

I am not sure what build system/compiler you are using; but it should NOT have compiled (or at a minimum produced a bunch of warnings) with your original format string. As a mater a fact, I needed to change your original string before it would be accepted by any of the 3 compilers mentioned above.

I tried white space before the %c, a good practice from the dark ages.

I understand it forces a skip of whitespace before the character grab. But I have never heard of it being called 'good' practice in my entire life; and I have been at this longer than I would like to admit. You either expect the whitespace or you don't.

Best Regards

[Scott.Bonomi]

Nvanote -

I appreciate that you took the time to reply. I can only wish that it had actually been helpful.

The second half of you statement means you understand C not. Leading white space in a C Format string does not require a space, but it does allow for white-space to be there. A Format string of "%c" should return the first character in the provided input, which my well be white space and of no interest. A Format string of " %c" should return the first non-white-space character in the provided input which is often of more interest. Having an input dialog which is not white space insensitive would not be ideal. The Cyber mainframe OS (NOS as I recall) instantiated that class of dialog, where exactly one space was required between arguments. It was less pleasant than the original makefile syntax which differentiated between spaces and tabs.

For the first half of your statement, you did not read my entire post. I did add the "h" length modifier as you and documentation indicate might help. It did not appear to. Having learned C with a copy of K&R in one hand and a bsd4.1 workstation in the other, the new length modifiers do not automatically come to mind. The compiler should be able to figure that out without help. I guess modern compilers are not as smart as the more mature ones. If you are running through a c++ compiler where the class of the object may change during execution, there could be some value in being able to force the size of the object written, but would that not then have to be a dynamic value not a fixed value. I am not sure if it would be appropriate to allow writing a larger value than the provided argument. That would allow the code to trash the stack fame.

I have been using the tensa-esp32-elf\esp-2019r2-8.2.0\xtensa-esp32-elf version compiler. It worked correctly on a PC, but that is a different compiler. It is of course possible that something has decided I want wide characters not characters and the useful part of that byte is being written somewhere else.

As far as your warning goes, I am not quite as well warned as you seem to think I should be.

C:\Users\sbonomi\Desktop\esp-idf\TurboII>idf.py clean
Checking Python dependencies...
Python requirements from C:\Users\sbonomi\Desktop\esp-idf\requirements.txt are satisfied.
Executing action: clean
Running ninja in directory c:\users\sbonomi\desktop\esp-idf\turboii\build
Executing "ninja clean"...
[1/1] Cleaning all built files...
Cleaning... 828 files.

C:\Users\sbonomi\Desktop\esp-idf\TurboII>idf.py build
Checking Python dependencies...
Python requirements from C:\Users\sbonomi\Desktop\esp-idf\requirements.txt are satisfied.
Executing action: all (aliases: build)
Running ninja in directory c:\users\sbonomi\desktop\esp-idf\turboii\build
Executing "ninja all"...
[189/825] Building C object esp-idf/soc/CMakeFiles/__idf_soc.dir/src/soc_include_legacy_warn.c.obj
C:/Users/sbonomi/Desktop/esp-idf/components/soc/src/soc_include_legacy_warn.c:4:2: warning: #warning Legacy including is enabled. This will be deprecated in the future. You can disable this option in the menuconfig. If there are some including errors, please try to include: "soc/soc.h", "soc/soc_memory_layout.h", "driver/gpio.h", or "esp_sleep.h". [-Wcpp]
#warning Legacy including is enabled. This will be deprecated in the future. You can disable this option in the menuconfig. If there are some including errors, please try to include: "soc/soc.h", "soc/soc_memory_layout.h", "driver/gpio.h", or "esp_sleep.h".
^~~~~~~
[450/825] Performing configure step for 'bootloader'
-- mconf-idf version mconf-v4.6.0.0-idf-20190628-win32
-- Project version: v4.0-dirty
-- Building ESP-IDF components for target esp32
-- Adding linker script C:/Users/sbonomi/Desktop/esp-idf/components/esp32/ld/esp32.peripherals.ld
-- Adding linker script C:/Users/sbonomi/Desktop/esp-idf/components/esp_rom/esp32/ld/esp32.rom.ld
-- Adding linker script C:/Users/sbonomi/Desktop/esp-idf/components/esp_rom/esp32/ld/esp32.rom.newlib-funcs.ld
-- Adding linker script C:/Users/sbonomi/Desktop/esp-idf/components/esp_rom/esp32/ld/esp32.rom.libgcc.ld
-- Adding linker script C:/Users/sbonomi/Desktop/esp-idf/components/bootloader/subproject/main/esp32.bootloader.ld
-- Adding linker script C:/Users/sbonomi/Desktop/esp-idf/components/bootloader/subproject/main/esp32.bootloader.rom.ld
-- Components: bootloader bootloader_support efuse esp32 esp_common esp_rom esptool_py log main micro-ecc partition_table soc spi_flash xtensa
-- Component paths: C:/Users/sbonomi/Desktop/esp-idf/components/bootloader C:/Users/sbonomi/Desktop/esp-idf/components/bootloader_support C:/Users/sbonomi/Desktop/esp-idf/components/efuse C:/Users/sbonomi/Desktop/esp-idf/components/esp32 C:/Users/sbonomi/Desktop/esp-idf/components/esp_common C:/Users/sbonomi/Desktop/esp-idf/components/esp_rom C:/Users/sbonomi/Desktop/esp-idf/components/esptool_py C:/Users/sbonomi/Desktop/esp-idf/components/log C:/Users/sbonomi/Desktop/esp-idf/components/bootloader/subproject/main C:/Users/sbonomi/Desktop/esp-idf/components/bootloader/subproject/components/micro-ecc C:/Users/sbonomi/Desktop/esp-idf/components/partition_table C:/Users/sbonomi/Desktop/esp-idf/components/soc C:/Users/sbonomi/Desktop/esp-idf/components/spi_flash C:/Users/sbonomi/Desktop/esp-idf/components/xtensa
-- Configuring done
-- Generating done
-- Build files have been written to: C:/Users/sbonomi/Desktop/esp-idf/TurboII/build/bootloader
[474/825] Performing build step for 'bootloader'
[1/2] Linking C executable bootloader.elf
[2/2] Generating binary image from built executable
esptool.py v2.8
Generated C:/Users/sbonomi/Desktop/esp-idf/TurboII/build/bootloader/bootloader.bin
[825/825] Generating binary image from built executable
esptool.py v2.8
Generated C:/Users/sbonomi/Desktop/esp-idf/TurboII/build/TurboII.bin

Project build complete. To flash, run this command:
C:\Users\sbonomi\.espressif\python_env\idf4.0_py3.7_env\Scripts\python.exe ..\components\esptool_py\esptool\esptool.py -p (PORT) -b 460800 --before default_reset --after hard_reset write_flash --flash_mode dio --flash_size detect --flash_freq 40m 0x1000 build\bootloader\bootloader.bin 0x8000 build\partition_table\partition-table.bin 0x10000 build\TurboII.bin
or run 'idf.py -p (PORT) flash'


Exactly which errors were you expecting me to get during compilation?

[nvannote]

Leading white space in a C Format string does not require a space, but it does allow for white-space to be there. A Format string of "%c" should return the first character in the provided input, which my well be white space and of no interest. A Format string of " %c" should return the first non-white-space character in the provided input which is often of more interest.

I fully understand the implications of the leading whitespace in the sscanf format (and what I said), not sure how you inferred otherwise. If you require sscanf to skip leading spaces, by all means, keep a space at the beginning of the format string.

Code: Select all

const char * ShortForm =" %c, 0x%hx, %hd, %hd, %hd"

The compiler should be able to figure that out without help. I guess modern compilers are not as smart as the more mature ones.

This has nothing to do with the compiler. Internally, the sscanf function has a format string and a variable argument lists of (what it interprets as) void pointers, that is all. It uses the format string in deduce the type and target size of the pointers.

If you are running through a c++ compiler

I am not. And C++ will not “save” someone from misusing a variable argument list.

That would allow the code to trash the stack fame.

And that is exactly what is happening here.

I have been using the tensa-esp32-elf\esp-2019r2-8.2.0\xtensa-esp32-elf version compiler. It worked correctly on a PC, but that is a different compiler. It is of course possible that something has decided I want wide characters not characters and the useful part of that byte is being written somewhere else.

I ran the same test on ESP-IDF v4.0.1 esp-2019r2-8.2.0/xtensa-esp32-elf-gcc, macOS/Darwin and Linux, and in all cases it failed with the original format string. But succeeded and ran correctly after modification. The original will corrupt the stack on any platform. Wether it is apparent or not would depend on the target architecture and how the compiler arranged the stack.

Exactly which errors were you expecting me to get during compilation?

Straight from a ESP-IDF v4.0.1 esp-2019r2-8.2.0/xtensa-esp32-elf-gcc compile. I suspect the difference is that I am passing the format string as a string literal and you are passing it as a variable.


../main/main.c:19:36: error: format '%x' expects argument of type 'unsigned int *', but argument 4 has type 'int16_t *' {aka 'short int *'} [-Werror=format=]
../main/main.c:19:40: error: format '%d' expects argument of type 'int *', but argument 5 has type 'int16_t *' {aka 'short int *'} [-Werror=format=]
../main/main.c:19:44: error: format '%d' expects argument of type 'int *', but argument 6 has type 'int16_t *' {aka 'short int *'} [-Werror=format=]
../main/main.c:19:48: error: format '%d' expects argument of type 'int *', but argument 7 has type 'int16_t *' {aka 'short int *'} [-Werror=format=]

The second half of you statement means you understand C not.

Seriously... Says the one misusing sscanf. Have a nice life.

Best Regards