Conversation
| if (ret == FAIL) { | ||
| COPY_CLIENT_ERROR(stmt->error_info, *conn->error_info); | ||
| DBG_RETURN(FAIL); | ||
| /* Don't return early - continue with cleanup to prevent memory leaks */ |
There was a problem hiding this comment.
If we do not return early here, won’t this end up returning PASS?
It introduces a bit of code duplication, but how about adding the cleanup logic here?
There was a problem hiding this comment.
Is this better now?
ndossche
left a comment
ndossche
left a comment
There was a problem hiding this comment.
I'm no longer involved, so can't meaningfully review this without spending time on it.
From a quick look it seems logical but who knows for sure without spending more time on it.
| break; | ||
| } | ||
| if (statistic != STAT_LAST) { | ||
| MYSQLND_INC_CONN_STATISTIC(conn->stats, statistic); |
There was a problem hiding this comment.
No knowledge of mysqlnd, but is it correct for MYSQLND_INC_CONN_STATISTIC to run in this case? Otherwise, a goto might help.
There was a problem hiding this comment.
I think it is correct. It doesn't matter much because it's only a statistic. Even if the prepared statement isn't freed on the server, everything related to it will be gone in PHP so updating the statistics makes sense. There is no way, IMHO, that you can call close again.
|
I think this should be testable using fake server. |
5 participants
No description provided.