[Enhancement/Feature Suggestion] Building while using printf() but not using stdio_init_all() should throw error. #1350

New issue

Open

opened 2023-04-24 00:37:56 -05:00 by GhostDog98 · 3 comments

GhostDog98 commented

2023-04-24 00:37:56 -05:00

(Migrated from github.com)

Take for example this code:

#include <stdio.h>
#include "pico/stdlib.h"

int main() {
    printf("Testing");
    return 1;
}

This does not work, but the user is provided no feedback on why it does not work, and are given no idea what to do to troubleshoot it.
I propose it should throw an error like:
"Warning: You have used printf without initialising stdio using stdio_init_all(); beforehand, this means that printf will silently error and not print anything to your computer."

It's very unintuitive to build a bit of code and for printf to not work for no discernable reason, only to learn that you need to include a specific line of code. The SDK should throw a warning or error to the user when attempting to build and the code contains printf, but not stdio_init_all. Take for example this code: ```c #include <stdio.h> #include "pico/stdlib.h" int main() { printf("Testing"); return 1; } ``` This does not work, but the user is provided no feedback on why it does not work, and are given no idea what to do to troubleshoot it. I propose it should throw an error like: "Warning: You have used printf without initialising stdio using `stdio_init_all();` beforehand, this means that printf will silently error and not print anything to your computer."

Gadgetoid commented

2023-04-25 06:52:18 -05:00

(Migrated from github.com)

Doing static checks on the source would be onerous, but maybe you could inspect the compiled symbols like so:

check.sh:

#!/bin/bash
MAP_FILE=$1

success() {
	echo -e "$(tput setaf 2)$1$(tput sgr0)"
}

inform() {
	echo -e "$(tput setaf 6)$1$(tput sgr0)"
}

warning() {
	echo -e "$(tput setaf 1)$1$(tput sgr0)"
}

if [[ "$MAP_FILE" == "" ]] then
    printf "Usage: $0 file.elf.map\n"
    exit 0
fi

if [ ! -f $MAP_FILE ]; then
    printf "Could not find map: $MAP_FILE.\n"
    exit 1
fi

grep -E '\(std::cout\)|\s__wrap_printf' $MAP_FILE > /dev/null
FOUND_PRINTF=$?

grep -E '\sstdio_init_all' $MAP_FILE > /dev/null
FOUND_STDIO_INIT=$?

grep -E '\sstdio_uart_init' $MAP_FILE > /dev/null
FOUND_STDIO_UART_INIT=$?

grep -E '\sstdio_usb_init' $MAP_FILE > /dev/null
FOUND_STDIO_USB_INIT=$?

if [[ $FOUND_PRINTF -eq 0 ]]; then
    inform "Found printf(); usage."
    if [[ $FOUND_STDIO_INIT -eq 1 ]]; then
        warning "WARNING: Found printf or std::cout with no stdio_init_all();."
        exit 1
    else
        inform "Found stdio_init_all();"
        if [[ $FOUND_STDIO_USB_INIT -eq 0 ]]; then
            inform "USB stdio is enabled."
        fi
        if [[ $FOUND_STDIO_UART_INIT -eq 0 ]]; then
            inform "UART stdio is enabled."
        fi
        if [[ $FOUND_STDIO_USB_INIT -eq 1 && $FOUND_STDIO_UART_INIT -eq 1 ]]; then
            warning "WARNING: No stdio output method enabled."
            printf "Try adding one of the following to CMakeLists.txt:\n"
            printf "pico_enable_stdio_uart(<target> 1).\n"
            printf "pico_enable_stdio_usb(<target> 1).\n"
            exit 1
        fi
    fi
fi

success "OK"

Add to CMakeLists.txt:

function(pico_check_stdio_usage TARGET)
	add_custom_command(TARGET ${TARGET} POST_BUILD
		COMMAND ${CMAKE_CURRENT_SOURCE_DIR}/check.sh ${TARGET}.elf.map
		WORKING_DIRECTORY ${CMAKE_CURRENT_BUILD_DIR}
		COMMENT "Running stdio check..."
	)
endfunction()

pico_check_stdio_usage(<YOUR_TARGET_NAME>)

This only checks for '$std::cout$|\s__wrap_printf' (std::cout and printf()) so it's by no means comprehensive, but at leasts suggests that such a helper is possible. Even if it is rather tricky!

Doing static checks on the source would be onerous, but maybe you could inspect the compiled symbols like so: `check.sh`: ```bash #!/bin/bash MAP_FILE=$1 success() { echo -e "$(tput setaf 2)$1$(tput sgr0)" } inform() { echo -e "$(tput setaf 6)$1$(tput sgr0)" } warning() { echo -e "$(tput setaf 1)$1$(tput sgr0)" } if [[ "$MAP_FILE" == "" ]] then printf "Usage: $0 file.elf.map\n" exit 0 fi if [ ! -f $MAP_FILE ]; then printf "Could not find map: $MAP_FILE.\n" exit 1 fi grep -E '$std::cout$|\s__wrap_printf' $MAP_FILE > /dev/null FOUND_PRINTF=$? grep -E '\sstdio_init_all' $MAP_FILE > /dev/null FOUND_STDIO_INIT=$? grep -E '\sstdio_uart_init' $MAP_FILE > /dev/null FOUND_STDIO_UART_INIT=$? grep -E '\sstdio_usb_init' $MAP_FILE > /dev/null FOUND_STDIO_USB_INIT=$? if [[ $FOUND_PRINTF -eq 0 ]]; then inform "Found printf(); usage." if [[ $FOUND_STDIO_INIT -eq 1 ]]; then warning "WARNING: Found printf or std::cout with no stdio_init_all();." exit 1 else inform "Found stdio_init_all();" if [[ $FOUND_STDIO_USB_INIT -eq 0 ]]; then inform "USB stdio is enabled." fi if [[ $FOUND_STDIO_UART_INIT -eq 0 ]]; then inform "UART stdio is enabled." fi if [[ $FOUND_STDIO_USB_INIT -eq 1 && $FOUND_STDIO_UART_INIT -eq 1 ]]; then warning "WARNING: No stdio output method enabled." printf "Try adding one of the following to CMakeLists.txt:\n" printf "pico_enable_stdio_uart(<target> 1).\n" printf "pico_enable_stdio_usb(<target> 1).\n" exit 1 fi fi fi success "OK" ``` Add to `CMakeLists.txt`: ``` function(pico_check_stdio_usage TARGET) add_custom_command(TARGET ${TARGET} POST_BUILD COMMAND ${CMAKE_CURRENT_SOURCE_DIR}/check.sh ${TARGET}.elf.map WORKING_DIRECTORY ${CMAKE_CURRENT_BUILD_DIR} COMMENT "Running stdio check..." ) endfunction() pico_check_stdio_usage(<YOUR_TARGET_NAME>) ``` This only checks for `'$std::cout$|\s__wrap_printf'` (`std::cout` and `printf()`) so it's by no means comprehensive, but at leasts suggests that such a helper is possible. Even if it is rather tricky!

rbehm-ibb commented

2023-04-27 03:46:23 -05:00

(Migrated from github.com)

Independent of the previous comment "throw error" is a bad idea. I usually only use stdio_usb_init(); and it would always fail.

GhostDog98 commented

2023-04-27 16:46:15 -05:00

(Migrated from github.com)

Independent of the previous comment "throw error" is a bad idea. I usually only use stdio_usb_init(); and it would always fail.

Maybe a more comprehensive whitelist would be needed (and throwing a warning instead), but the idea is still there of helping to avoid a rather obscure silent error.

> Independent of the previous comment "throw error" is a bad idea. I usually only use stdio_usb_init(); and it would always fail. Maybe a more comprehensive whitelist would be needed (and throwing a warning instead), but the idea is still there of helping to avoid a rather obscure silent error.