hnakamur commented on PR #12167: URL: https://github.com/apache/trafficserver/pull/12167#issuecomment-2786343180
> Here's the clang-analyzer report concerning a nullptr dereference, in case it's helpful: > > https://ci.trafficserver.apache.org/job/Github_Builds/job/clang-analyzer/5704/artifact/output/12167/scan-build-2025-04-07-13-36-30-195012-dqlezm4f/index.html I suppose this was a false positive from clang-analyzer. I did 'git bisect' with a script whose content is https://github.com/apache/trafficserver-ci/blob/9a6bb8b40f2a6c830d52f619fa06cda22c60abf4/jenkins/github/clang-analyzer.pipeline#L60-L91 and found the commit https://github.com/apache/trafficserver/pull/12167/commits/99a15bfab85b23535e71b1882b0dede635f1be3e was the trigger. However I did not think the commit was actual cause after reviewing the diff. I think clang-analyzer was confused that `string_get` may return nullptr because of `if (new_url == nullptr)` below it. ``` new_url = to_free = u->string_get(&s->arena, &new_url_len); if (new_url == nullptr) { new_url = ""; } ``` I suppose that `string_get` always returns non nullptr. So I added `assert(to_free != nullptr);` at https://github.com/apache/trafficserver/pull/12167/commits/038258a62b5a3663ea1aa68e443866da02552419 and clang-analyzer does not report nullptr dereference anymore. If this assumption is true, the following three lines are not needed. ``` if (new_url == nullptr) { new_url = ""; } ``` If this assumption is false, we must guard the call to `str_free` like below: ``` if (to_free) { s->arena.str_free(to_free); } ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@trafficserver.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
