Hello, I just noticed that there are some problems with the usage of asprintf() throughout the code base. This function is not properly checked for failure, which can lead to nasty crashes.
I am checking here: https://linux.die.net/man/3/asprintf It states: > Return Value When successful, these functions return the number of bytes > printed, just like *sprintf <https://linux.die.net/man/3/sprintf>(3)*. If > memory allocation wasn't possible, or some other error occurs, these > functions will return -1, and the contents of *strp* is undefined. > Notice that in case of failure, the contents of strp will be undefined. To my knowledge, the only proper way to check for failure is to check for a negative return value. Indeed the NuttX version of this function follows this description. It will always return -1 in case of error. The provided pointer will not be touched, unless the function has succeeded. However, in several cases, the users of asprintf make the assumption that the pointer will be set to NULL in case of failure! (Which is definitely not the case!) For example see libs/libc/stdio/lib_tempnam.c. In case of any error (malloc failure for example), the function will continue with a garbage value in variable `template`. (Neither the variable is initialized, nor the return value of asprintf is checked). There are many more such cases throughout NuttX. So, this must be fixed. This serious flaw affects several parts of NuttX, including the standard library, the file system and maybe more. The question is how? The straightforward method would be to search for all uses of asprintf() and refactor them to properly check the return value. I consider this the most "correct" and portable approach, but it is quite some work. Another idea is to change asprintf() to set the pointer to NULL in case of failure. We decide that this is how the NuttX's version of asprintf() works, and we satisfy the assumption of the callers. I think that this will instantly solve all problems. FInally, we can just make sure that the provided pointer is always initialized to NULL. Since it will not be touched in case of failure, we can move on with minimal effort. I am not sure about portability though... What do you think?
