Xtranssock.c: fix format truncation warning #1405
Conversation
|
Not sure if this is a good thing, asprintf will allocate some heap memory uselessly here, where the snprintf wouldn't. Also your code is probably wrong: strncpy(path, buf, sizeof(s.sun_path-1));
// should be:
strncpy(path, buf, sizeof(s.sun_path) - 1);And strncpy(path, buf, maxlen); |
This is unfortunately true, but I am not sure there is a better way of convincing the compiler we know what we are doing here other than simply silencing the warning.
Fixed, thank you. The commit now passes all the cases and is ready for review.
The compiler can optimize this into a constant instead of having to access memory which may take a number of cpu cycles, unless you make it constant. |
I could try using strcat and then strcopy to get around this, but we will still need to make a buffer on the stack that has the same lifetime as the asprintf buffer... |
|
If there are no further issues with this commit I would like it merged so that we can start thinking about setting -Wextra or -Werror |
| snprintf(path, sizeof(s.sun_path), "%s%s%s", at, upath, port); | ||
|
|
||
| asprintf(&buf, "%s%s%s", at, upath, port); | ||
| strncpy(path, buf, sizeof(s.sun_path)-1); |
There was a problem hiding this comment.
This is doing needless copies and filling the buffer with extra 0 bytes.
Don't try to set -Wextra on the X server, there are lot's of warnings there that don't indicate any issue, and fixing those would just waste a lot of time while making the code unnatural and harder to read. About -Werror, I'm planning to set it in CI after the openssl deprecation warnings get fixed. |
3 participants
I was tired of staring at this every compile, since we check if the size is appropriate before we write, we can use asprintf and the compiler now trusts that we know what we are doing, and if we didn't we returned with -1 before it could allocate.