gpu/utils: fix null pointer arithmetic in mp_log_source()#17715
gpu/utils: fix null pointer arithmetic in mp_log_source()#17715PatriciaBucataru wants to merge 1 commit intompv-player:masterfrom
Conversation
This sounds as if we had invalid null check, when in fact the |
Understood, do you want me to update the commit message? |
| if (!end) | ||
| next = end = src + strlen(src); | ||
| else | ||
| next = end + 1; |
There was a problem hiding this comment.
please add braces around the if-else
see: https://github.com/mpv-player/mpv/blob/master/DOCS/contribute.md
PatriciaBucataru
force-pushed
the
fix/null-ptr-arithmetic-mp-log-source
branch
from
c7705f6 to
a22f2b3
Compare
Incorrect. Both gcc and clang have null pointer "optimization" which can optimize out the |
Citation needed. I don't believe you such aggressive optimization is implemented in GCC, I would guess LLVM at some point could do it, but it also was tamed over the years. And remember we are talking about the snippet of code in this PR, not some other theoretical case. |
Hmm, looks like their null-pointer optimization currently only activates on dereferences. Not on invalid pointer arithmetic. Yet at least, such optimization would be permitted by the standard if they want to do it. Anyhow, the current commit message is completely wrong @PatriciaBucataru. It's not "adding braces", that's just stylistic, it's fixing a UB. |
Sure, but will never happen. This would be breaking change and you cannot break majority of codebases. Also the point for UB optimization is to avoid "doing" something which is implicitly assumed by standard. For example in this case, it is already optimized into conditional move, with or without this PR, so anything else would be deoptimization. |
strchr() can return NULL when no newline is found. Computing end + 1 before the null check on end is undefined behaviour per C11 6.5.6. Move the next assignment inside the else branch to avoid the UB.
PatriciaBucataru
force-pushed
the
fix/null-ptr-arithmetic-mp-log-source
branch
from
a22f2b3 to
630a0e7
Compare
Similar thing has happened countless times to the Linux kernel to the point that Linux is basically using its own dialect of C requiring lots of specific GCC compiler flags. |
Similar, not the same. Compiler will never do more work in the name of UB, it is always the other way around. |
5 participants
strchr() can return NULL when no newline is found. next was computed as end + 1 before the null check on the line below, which is undefined behaviour per C11 6.5.6.
Moved the next assignment after the null check.