locale.c: Don't do asymmetric back out on failure #23172
Conversation
khwilliamson
force-pushed
the
smoke-me/khw-darwin_locale
branch
from
eb43f7f to
b9e1bad
Compare
|
It's not clear to me if |
|
There's a few paths where they can leak. |
|
khwilliamson
force-pushed
the
smoke-me/khw-darwin_locale
branch
from
b9e1bad to
a25786b
Compare
|
I've reexamined this commit, and don't see anything wrong with it. It does look to me that the first two goto's can be replaced by returns, as nothing has been allocated yet. But I don't see any leak paths. @tonycoz can you provide a case |
| @@ -10090,7 +10090,7 @@ Perl_mem_collxfrm_(pTHX_ const char *input_string, | |||
| if (UNLIKELY(! xbuf)) { | |||
| DEBUG_L(PerlIO_printf(Perl_debug_log, | |||
| "mem_collxfrm_: Couldn't malloc %zu bytes\n", xAlloc)); | |||
| goto bad; | |||
| goto bad_no_strxfrm; | |||
There was a problem hiding this comment.
This looks like a potential memory leak to me. At this point, sans_highs, sans_nuls, and free_me may have been allocated, but bad_no_strxfrm skips over the free() calls.
There was a problem hiding this comment.
Somehow I got it into my head that that label had code to do the necessary cleanup, and was blind to the fact that it didn't. But with the latest push, it should now.
This is in preparation for the next commit where it will be split out to be a stand-alone macro.
This fixes #23519 When something goes wrong doing locale-aware string collation, the code attempts to carry on as well as can be expected. Prior to this commit the backout code was asymmetric, trying to undo things that had not been done. This happened when the failure was early on. In the case of this ticket, the platform has a defective locale that was detectable before getting very far along. The solution adopted here is to jump to a different label for those early failures that does less backout than for later failures.
This changes a subsidiary function's return value from void to bool, returning false if it finds the locale doesn't have sane collation. The calling function is changed to check this, and give up immediately if the locale isn't sane.
The previous commit removed a block; so can outdent
khwilliamson
force-pushed
the
smoke-me/khw-darwin_locale
branch
from
6449b82 to
13d2b29
Compare
|
Test suite still passes on Tiger/i386 with the latest version of the patch applied. |
This fixes #23159
When something goes wrong doing locale-aware string collation, the code attempts to carry on as well as can be expected. prior to this commit the backout code was asymmetric, trying to undo things that had not been done. This happened when the failure was early on.
In the case of this ticket, the platform has a defective locale that was detectable before getting very far along.
The solution adopted here is to jump to a different label for those early failures that does less backout than for later failures.