Esp32-hal-uart.c:153:9: warning: 'return' with no value

Hardware Nano Family Nano ESP32
1

Hi,

I tried Arduino IDE 2.2.1 with Manjaro.
Why is Pyserial not installed together with the IDE? Because that is a requirement.

Compiled an empty sketch. That works so far.
But the esp32-hal-uart.c gives an unpleasant warning.

/home/user/.arduino15/packages/arduino/hardware/esp32/2.0.13/cores/esp32/esp32-hal-uart.c: In function 'uartSetPins':
/home/user/.arduino15/packages/arduino/hardware/esp32/2.0.13/cores/esp32/esp32-hal-uart.c:153:9: warning: 'return' with no value, in function returning non-void
         return;
         ^~~~~~
/home/user/.arduino15/packages/arduino/hardware/esp32/2.0.13/cores/esp32/esp32-hal-uart.c:149:6: note: declared here
 bool uartSetPins(uint8_t uart_num, int8_t rxPin, int8_t txPin, int8_t ctsPin, int8_t rtsPin)
      ^~~~~~~~~~~

Line 153: Surely this should return false? According to the comment, -1 should also be returned. However, this does not work with bool.

// Valid pin UART_PIN_NO_CHANGE is defined to (-1)
// Negative Pin Number will keep it unmodified, thus this function can set individual pins
bool uartSetPins(uint8_t uart_num, int8_t rxPin, int8_t txPin, int8_t ctsPin, int8_t rtsPin)
{
    if(uart_num >= SOC_UART_NUM) {
        log_e("Serial number is invalid, please use numers from 0 to %u", SOC_UART_NUM - 1);
        return;
    }

    // IDF uart_set_pin() will issue necessary Error Message and take care of all GPIO Number validation.
    bool retCode = uart_set_pin(uart_num, txPin, rxPin, rtsPin, ctsPin) == ESP_OK; 
    return retCode;
}

Please correct.

I am looking for the file esp32-hal-uart.c on Github-Arduino and cannot find it. I only find the github espressif esp32-hal-uart.c
with different content. But the function uartSetPins() on line 291 with return false.
Is the core from Espressif better than the one from Arduino?

2

Hi @Doc_Arduino. The "esp32" boards platform developers are tracking this defect here:

If you have a GitHub account, you can subscribe to that issue to get notifications of any new developments related to this subject:

image

Here it is:

https://github.com/arduino/arduino-esp32/blob/arduino_nano_esp32/cores/esp32/esp32-hal-uart.c

The only difference between the releases of the "Arduino ESP32 Boards" platform and the "esp32" boards platform is that support for all boards other than the Nano ESP32 is stripped out.

The reason Arduino maintains a fork of the platform for the Nano ESP32 board is to provide a slightly more polished user experience for those using a Nano ESP32. The "esp32" platform has support for a large number of boards, which is awesome, but also might be overwhelming for a less experienced user. So for someone who only uses the Nano ESP32 board, I would say the "Arduino ESP32 Boards" platform is slightly better. But if you are using multiple different models of ESP32 boards or microcontrollers then the "esp32" platform will be better.

Something to note is that all development is done in the espressif/arduino-esp32 repository. So if you are interested in submitting bug reports or code contributions to the platform, or beta testing the latest development version, you should use that repository.

3

Hi,

okay. so far I have understood pretty well. Thank you.

But why does my esp32-hal-uart.c on my computer have a different content than the Arduino esp32-hal-uart.c on Github?

github.com/arduino/arduino-esp32/blob/arduino_nano_esp32/cores/esp32/esp32-hal-uart.c

Should I replace the file myself?

4

The file on your computer is from the 2.0.13 release of the platform. Try comparing the file at the 2.0.13 tag in the repository:

https://github.com/arduino/arduino-esp32/blob/2.0.13/cores/esp32/esp32-hal-uart.c

As far as I know, the problem hasn't been fixed yet (they should close issue espressif/arduino-esp32#8641 when it is) so I don't expect that you can fix the warning by replacing the local files with some files from GitHub.

You are welcome submit a pull request for a fix to the the espressif/arduino-esp32 repository.

For bonus points, also submit the change to the continuous integration ("CI") system to promote such warnings to errors so that similar regressions can not be introduced in the future:

https://github.com/espressif/arduino-esp32/issues/8641#issuecomment-1722310639

I was wondering if we need to increase the warning-error check setting in CI.
These messages should be picked up in the CI.

If you don't want to do that then I recommend you to simply ignore the warning and get on with your project.

Are you experiencing some problem you think is related to the lack of a return value in this code? Or is it only that you are bothered by compiler warnings (which is completely understandable)?

5

Hi,

I am bothered by the warning. And I wonder why this warning was overlooked.

I'll see if I can manage the pull request. :wink:

6

Hi,

I have noticed something together with other topics in the forum. Why have you made the compiler settings stricter? Harmless warnings are always hard errors. Every unused warning aborts the compilation. This can affect all foreign libraries. Like the return error here, which could also be a warning. This causes more problems than it helps. If someone sets this up more strictly for themselves, that is something else. They certainly know what they are doing.

ESP32 platform.txt.

# Arduino Compile Warning Levels
compiler.warning_flags=-w
compiler.warning_flags.none=-w
compiler.warning_flags.default=
compiler.warning_flags.more=-Wall -Werror=all
compiler.warning_flags.all=-Wall -Werror=all -Wextra
7

The problem is that a lot of developers are very sloppy and lazy. If you don't force these developers to write quality code then they won't. And then we end up with compilation output that is a huge wall of red text from all the warnings in code we didn't write. That makes it difficult and annoying for those of us who hold our own code to higher standards to even spot the warnings produced by the code we wrote.

I think the developers of the "esp32" boards platform chose to configure it this way in order to encourage people to write higher quality code. I think the intention behind it is commendable. I am certain that it has indeed resulted in a higher quality of code in the ESP32 Arduino ecosystem. However, there are some downsides:

  • The type of developer who writes code that generates warnings is also the type of developer who doesn't enable warnings in the IDE settings. This means system doesn't result in them writing higher quality code off the bat (though they might eventually be forced to fix it when the users report it doesn't compile).
  • The users are even more significantly impacted by the sloppy code written by others.
8

Hi,

okay, I understand the point of view. But then I have to ask how the esp32-hal-uart.c lib with no return error could be released with the stricter setting. Everyone from the Arduino team should have noticed that. :wink:

However. I wish you a Merry Christmas and a Happy New Year.

9

The system that promotes warnings to errors doesn't promote all types of warnings to errors. This particular warning is not an error even when you have the warnings fully enabled in the IDE. So the system for enforcing the creation of warning free code doesn't apply here.

I'm not sure what you mean by "Arduino team", but just to be clear the "esp32" boards platform is a 3rd party project not maintained by the Arduino company. Other than the removal of the board definitions, Arduino's "Arduino ESP32 Boards" platform is an exact copy of the code from the "esp32" boards platform.

10

Hi,

Warnung vs. Error

Upps, sorry, I mistook it.

Core

I got that mixed up too. Sorry. I really thought Arduino maintains its own core based only on the original. Good to know that it is a 1:1 copy. Thanks.

11

I did a bit of experimentation and found that the behavior of "'return' with no value, in function returning non-void" being a warning is specific to C. If I write some code with this problem in a C++ or .ino file, then it is treated as an error regardless of the warning setting:

SomeSketch.ino:

void setup() {}
void loop() {}
int foo() {
  return;
}

SomeSketch/SomeSketch.ino: In function 'int foo()':
SomeSketch/SomeSketch.ino:4:3: error: return-statement with no value, in function returning 'int' [-fpermissive]
 }
   ^     

exit status 1

Compilation error: return-statement with no value, in function returning 'int' [-fpermissive]

So this is fundamentally considered an error by the C++ compiler (except in the case where the compilation command uses the -fpermissive flag to downgrade the error to a warning, which is never done by the ESP32 platform).

But the same code in a C file only produces a warning:

SomeSketch.ino:

void setup() {}
void loop() {}

SomeC.c:

int foo() {
  return;
}

SomeSketch/SomeC.c: In function 'foo':
SomeSketch/SomeC.c:2:3: warning: 'return' with no value, in function returning non-void
SomeSketch/SomeC.c:1:5: note: declared here

It is not a matter of any warning in a C file not being promoted because other warnings are promoted as expected.

If I change the -Werror=all flag to -Werror then the warning in the C file is promoted to an error as expected.

I didn't expect this warning to be handled differently by the same compiler from one language to another and didn't find anything to explain it in the GCC documentation.

12

Hi,

yes, this should actually be equally valid for C and C++. I will also take a closer look at this.

13

Hi,

before I forget. :slight_smile:
Results are.

main.cpp always compiles with error regardless of the option and regardless of the toolchain.

main.c compiles depending on the option and depending on the toolchain.

Programm in main.c / main.cpp 
#define F_CPU 16000000UL

#include <avr/io.h>
#include <util/delay.h>
#include <avr/interrupt.h>
#include <util/atomic.h>

int foo (void)
{
  return;
}

int main(void)
{      
    while (1)
    { }
}
Results

--- avr-gcc-8.5.0 binutils2.40 avrLibc2.1 ---
--- main.c ---

avr-gcc-8.5.0.exe -Wall -Os main.c -o main.elf -Os -mmcu=atmega328P
Warning

avr-gcc-8.5.0.exe -Wall -Os main.c -o main.elf -Os -mmcu=atmega328P -Wextra
Warning

avr-gcc-8.5.0.exe -Wall -Os main.c -o main.elf -Os -mmcu=atmega328P -Werror
Error

avr-gcc-8.5.0.exe -Wall -Os main.c -o main.elf -Os -mmcu=atmega328P -Werror=all
Warning

--- main.cpp ---

avr-gcc-8.5.0.exe -Wall -Os main.cpp -o main.elf -Os -mmcu=atmega328P
Error

avr-gcc-8.5.0.exe -Wall -Os main.cpp -o main.elf -Os -mmcu=atmega328P -Wextra
Error

avr-gcc-8.5.0.exe -Wall -Os main.cpp -o main.elf -Os -mmcu=atmega328P -Werror
Error

avr-gcc-8.5.0.exe -Wall -Os main.cpp -o main.elf -Os -mmcu=atmega328P -Werror=all
Error

--- avr-gcc-13.2.0 binutils2.41 avrLibc2.1 ---
--- main.c ---

avr-gcc-13.2.0.exe -Wall -Os main.c -o main.elf -Os -mmcu=atmega328P
Warning

avr-gcc-13.2.0.exe -Wall -Os main.c -o main.elf -Os -mmcu=atmega328P -Wextra
Warning

avr-gcc-13.2.0.exe -Wall -Os main.c -o main.elf -Os -mmcu=atmega328P -Werror
Error

avr-gcc-13.2.0.exe -Wall -Os main.c -o main.elf -Os -mmcu=atmega328P -Werror=all
Error

--- main.cpp ---

avr-gcc-13.2.0.exe -Wall -Os main.cpp -o main.elf -Os -mmcu=atmega328P
Error

avr-gcc-13.2.0.exe -Wall -Os main.cpp -o main.elf -Os -mmcu=atmega328P -Wextra
Error

avr-gcc-13.2.0.exe -Wall -Os main.cpp -o main.elf -Os -mmcu=atmega328P -Werror
Error

avr-gcc-13.2.0.exe -Wall -Os main.cpp -o main.elf -Os -mmcu=atmega328P -Werror=all
Error

14

Thanks for sharing that information. I think the result for C with the -Werror=all flag aligning with the result with the -Werror flag when using the modern GCC version is an improvement.

By the way, in case you or other readers aren't already aware of it, I'll mention the very cool "Compiler Explorer" website, which is useful for this type of work:

15

Hi,

That's good too. :+1:
As a supplement to the topic wandbox. Example