Skip to content

Commit 12b48b9

Browse files
committed
Address review feedback: use bailout flag instead of duplicating cleanup
Use a bool flag in zend_catch and let the existing out: cleanup path handle resource freeing, then conditionally call zend_bailout() after. Also fix test to use short array syntax and remove stray blank line.
1 parent 4afdcf5 commit 12b48b9

2 files changed

Lines changed: 12 additions & 23 deletions

File tree

ext/iconv/iconv.c

Lines changed: 10 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -982,8 +982,8 @@ static php_iconv_err_t _php_iconv_mime_encode(smart_str *pretval, const char *fn
982982

983983
char_cnt = max_line_len;
984984

985+
bool bailout = false;
985986
zend_try {
986-
987987
_php_iconv_appendl(pretval, fname, fname_nbytes, cd_pl);
988988
char_cnt -= fname_nbytes;
989989
smart_str_appendl(pretval, ": ", sizeof(": ") - 1);
@@ -1191,19 +1191,7 @@ static php_iconv_err_t _php_iconv_mime_encode(smart_str *pretval, const char *fn
11911191
smart_str_0(pretval);
11921192

11931193
} zend_catch {
1194-
if (cd != (iconv_t)(-1)) {
1195-
iconv_close(cd);
1196-
}
1197-
if (cd_pl != (iconv_t)(-1)) {
1198-
iconv_close(cd_pl);
1199-
}
1200-
if (encoded != NULL) {
1201-
zend_string_release_ex(encoded, 0);
1202-
}
1203-
if (buf != NULL) {
1204-
efree(buf);
1205-
}
1206-
zend_bailout();
1194+
bailout = true;
12071195
} zend_end_try();
12081196

12091197
out:
@@ -1219,6 +1207,9 @@ static php_iconv_err_t _php_iconv_mime_encode(smart_str *pretval, const char *fn
12191207
if (buf != NULL) {
12201208
efree(buf);
12211209
}
1210+
if (bailout) {
1211+
zend_bailout();
1212+
}
12221213
return err;
12231214
}
12241215
/* }}} */
@@ -1258,6 +1249,7 @@ static php_iconv_err_t _php_iconv_mime_decode(smart_str *pretval, const char *st
12581249
}
12591250

12601251
p1 = str;
1252+
bool bailout = false;
12611253
zend_try {
12621254
for (str_left = str_nbytes; str_left > 0; str_left--, p1++) {
12631255
int eos = 0;
@@ -1766,13 +1758,7 @@ static php_iconv_err_t _php_iconv_mime_decode(smart_str *pretval, const char *st
17661758
smart_str_0(pretval);
17671759

17681760
} zend_catch {
1769-
if (cd != (iconv_t)(-1)) {
1770-
iconv_close(cd);
1771-
}
1772-
if (cd_pl != (iconv_t)(-1)) {
1773-
iconv_close(cd_pl);
1774-
}
1775-
zend_bailout();
1761+
bailout = true;
17761762
} zend_end_try();
17771763

17781764
out:
@@ -1782,6 +1768,9 @@ static php_iconv_err_t _php_iconv_mime_decode(smart_str *pretval, const char *st
17821768
if (cd_pl != (iconv_t)(-1)) {
17831769
iconv_close(cd_pl);
17841770
}
1771+
if (bailout) {
1772+
zend_bailout();
1773+
}
17851774
return err;
17861775
}
17871776
/* }}} */

ext/iconv/tests/gh17399.phpt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,9 @@ GH-17399 (iconv memory leak with large line-length in iconv_mime_encode)
44
iconv
55
--FILE--
66
<?php
7-
$options = array(
7+
$options = [
88
'line-length' => PHP_INT_MAX,
9-
);
9+
];
1010
iconv_mime_encode('Subject', 'test', $options);
1111
?>
1212
--EXPECTF--

0 commit comments

Comments
 (0)