-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
STM32L-Discovery (STM32L152R8T6) can't write firmware to chip with st-flash on Windows 10 #1214
Comments
To me it makes absolutely no sense to define |
Never turn to negative? Sorry, but can't agree. Many functions return -1 in case of errors. For example, send_recv from #909 mentioned by you. Refactor them all is too difficult - at least for me, these are my first steps in programming. |
@hydroconstructor I think this check if (sizeof(st.st_size) != sizeof(size_t)) {
// on 32 bit systems, check if there is an overflow
if (st.st_size > (off_t)SSIZE_MAX) {
fprintf(stderr, "mmap() size_t overflow for file %s\n", path);
goto on_error;
}
} may be replaced by if (st.st_size > (1<<20) /*1GB*/) {
// limit file size to 1GB
fprintf(stderr, "mmap() file %s too big\n", path);
goto on_error;
} The sizes of the MCU firmware will not go beyond this size for a long time, while we will eliminate the wrong choice of files. |
Good idea. Doing this just now... |
@hydroconstructor One needs to distinguish between functions and variables. I was talking about types for variables. |
@Nightwalker-87, functions have types. Type of send_recv from #909 is ssize_t, and it returns -1 in case of error. There are other functions with type ssize_t, returning -1 in case of error. If I make global change from |
@Nightwalker-87 Here the problem is in the specific library libc, which has a non-standard type st_size. An error occurs due to the different dimension and the С types conversion. |
@Ant-ON What would you propose as the most reasonable approach to deal with this? |
@hydroconstructor In general yes, but we need to be careful regarding possible side effects. As many contributors have worked on or modified the code there is no clear concept regarding this and nobody really seems to completely overlook the current state. This is also one reason why #909 was opened. It is clear that this topic wants some work being done, but I'm not sure how far we should go here within this ticket. Therefore I asked @Ant-ON for his opinion on this. |
Error in file size comparizon. Due to type casting, instead of compare file size with max. singed int value, it compares with -1. Then function returns with error message.
@Nightwalker-87 This problem is similar to #909 but not related to #909. In this case, the problem is in the 3th party library. |
Ok, fine - I noticed it has already been pushed by @hydroconstructor as well. 👍 |
From this point of view, max Back to the original issue: we have to check if our file size, if (st.st_size > SIZE_MAX) {/*…*/} // C99 stdint.h This way we would probably get a compiler warning, because if (st.st_size > (intmax_t) SIZE_MAX) {/*…*/} // C99 stdint.h This version will work as long as I’m sorry for coming here so late (now that there is a ready to ship PR) and I’m sorry if I have missed something obvious and we cannot actually use the above solution. HTH. |
@gszy That's ok, don't worry. Just proceed as intended. |
st-flash
Commandline output:
mmap() size_t overflow for file mb.bin
Expected/description:
Debugging revealed that problem is in file common.c, function map_file, in code below:
Type of st.st_size is off_t, which means long int. Value is size of firmware file and for me is 602 bytes.
SSIZE_MAX defines somewhere in libc headers. It means maximum size of signed integer. But in my system it compiled in long long int. Not 4 bytes long int as off_t, but 8 bytes long long int! So there is overflow in casting types without warnings during compile. As a result, instead of:
if (602 > 2^32-1) {
I got:
if (602 > -1) {
And then function returned with error message.
Proposed solution, which works on my computer:
In header section of common.c define macro OFF_T_MAX such a way:
In map_file function replace SSIZE_MAX by newly defined OFF_T_MAX.
Also please note that SSIZE_MAX defined in unistd.h in such a way:
There is an error in numbers. SSIZE_MAX is max. value of signed int and must be equal 0x400... . But here it equals 0x7FFF ... in both cases, which is true for UNsigned int. For signed int both values are -1. But in my system SSIZE_MAX defined in libc, so this macro is not used.
The text was updated successfully, but these errors were encountered: