Skip to content

Commit 2ac5948

Browse files
GirgiasGirgias
authored
Refactor SplFileObject CSV methods (#8317)
Use early returns, which fixes ValueError order of arguments and flattens the code
1 parent 3931d72 commit 2ac5948

File tree

2 files changed

+84
-93
lines changed

2 files changed

+84
-93
lines changed

‎ext/spl/spl_directory.c

Lines changed: 78 additions & 93 deletions
Original file line numberDiff line numberDiff line change
@@ -2331,35 +2331,32 @@ PHP_METHOD(SplFileObject, fgetcsv)
23312331

23322332
CHECK_SPL_FILE_OBJECT_IS_INITIALIZED(intern);
23332333

2334-
switch (ZEND_NUM_ARGS()) {
2335-
case 3:
2336-
if (esc_len > 1) {
2337-
zend_argument_value_error(3, "must be empty or a single character");
2334+
if (delim) {
2335+
if (d_len != 1) {
2336+
zend_argument_value_error(1, "must be a single character");
23382337
RETURN_THROWS();
23392338
}
2340-
if (esc_len == 0) {
2341-
escape = PHP_CSV_NO_ESCAPE;
2342-
} else {
2343-
escape = (unsigned char) esc[0];
2344-
}
2345-
ZEND_FALLTHROUGH;
2346-
case 2:
2339+
delimiter = delim[0];
2340+
}
2341+
if (enclo) {
23472342
if (e_len != 1) {
23482343
zend_argument_value_error(2, "must be a single character");
23492344
RETURN_THROWS();
23502345
}
23512346
enclosure = enclo[0];
2352-
ZEND_FALLTHROUGH;
2353-
case 1:
2354-
if (d_len != 1) {
2355-
zend_argument_value_error(1, "must be a single character");
2347+
}
2348+
if (esc) {
2349+
if (esc_len > 1) {
2350+
zend_argument_value_error(3, "must be empty or a single character");
23562351
RETURN_THROWS();
23572352
}
2358-
delimiter = delim[0];
2359-
ZEND_FALLTHROUGH;
2360-
case 0:
2361-
break;
2353+
if (esc_len == 0) {
2354+
escape = PHP_CSV_NO_ESCAPE;
2355+
} else {
2356+
escape = (unsigned char) esc[0];
2357+
}
23622358
}
2359+
23632360
if (spl_filesystem_file_read_csv(intern, delimiter, enclosure, escape, return_value) == FAILURE) {
23642361
RETURN_FALSE;
23652362
}
@@ -2378,49 +2375,41 @@ PHP_METHOD(SplFileObject, fputcsv)
23782375
zval *fields = NULL;
23792376
zend_string *eol = NULL;
23802377

2381-
if (zend_parse_parameters(ZEND_NUM_ARGS(), "a|sssS", &fields, &delim, &d_len, &enclo, &e_len, &esc, &esc_len, &eol) == SUCCESS) {
2378+
if (zend_parse_parameters(ZEND_NUM_ARGS(), "a|sssS", &fields, &delim, &d_len, &enclo, &e_len, &esc, &esc_len, &eol) == FAILURE) {
2379+
RETURN_THROWS();
2380+
}
23822381

2383-
switch(ZEND_NUM_ARGS())
2384-
{
2385-
case 5:
2386-
case 4:
2387-
switch (esc_len) {
2388-
case 0:
2389-
escape = PHP_CSV_NO_ESCAPE;
2390-
break;
2391-
case 1:
2392-
escape = (unsigned char) esc[0];
2393-
break;
2394-
default:
2395-
zend_argument_value_error(4, "must be empty or a single character");
2396-
RETURN_THROWS();
2397-
}
2398-
ZEND_FALLTHROUGH;
2399-
case 3:
2400-
if (e_len != 1) {
2401-
zend_argument_value_error(3, "must be a single character");
2402-
RETURN_THROWS();
2403-
}
2404-
enclosure = enclo[0];
2405-
ZEND_FALLTHROUGH;
2406-
case 2:
2407-
if (d_len != 1) {
2408-
zend_argument_value_error(2, "must be a single character");
2409-
RETURN_THROWS();
2410-
}
2411-
delimiter = delim[0];
2412-
ZEND_FALLTHROUGH;
2413-
case 1:
2414-
case 0:
2415-
break;
2382+
if (delim) {
2383+
if (d_len != 1) {
2384+
zend_argument_value_error(2, "must be a single character");
2385+
RETURN_THROWS();
24162386
}
2417-
2418-
ret = php_fputcsv(intern->u.file.stream, fields, delimiter, enclosure, escape, eol);
2419-
if (ret < 0) {
2420-
RETURN_FALSE;
2387+
delimiter = delim[0];
2388+
}
2389+
if (enclo) {
2390+
if (e_len != 1) {
2391+
zend_argument_value_error(3, "must be a single character");
2392+
RETURN_THROWS();
24212393
}
2422-
RETURN_LONG(ret);
2394+
enclosure = enclo[0];
24232395
}
2396+
if (esc) {
2397+
if (esc_len > 1) {
2398+
zend_argument_value_error(4, "must be empty or a single character");
2399+
RETURN_THROWS();
2400+
}
2401+
if (esc_len == 0) {
2402+
escape = PHP_CSV_NO_ESCAPE;
2403+
} else {
2404+
escape = (unsigned char) esc[0];
2405+
}
2406+
}
2407+
2408+
ret = php_fputcsv(intern->u.file.stream, fields, delimiter, enclosure, escape, eol);
2409+
if (ret < 0) {
2410+
RETURN_FALSE;
2411+
}
2412+
RETURN_LONG(ret);
24242413
}
24252414
/* }}} */
24262415

@@ -2433,43 +2422,39 @@ PHP_METHOD(SplFileObject, setCsvControl)
24332422
char *delim = NULL, *enclo = NULL, *esc = NULL;
24342423
size_t d_len = 0, e_len = 0, esc_len = 0;
24352424

2436-
if (zend_parse_parameters(ZEND_NUM_ARGS(), "|sss", &delim, &d_len, &enclo, &e_len, &esc, &esc_len) == SUCCESS) {
2437-
switch(ZEND_NUM_ARGS())
2438-
{
2439-
case 3:
2440-
switch (esc_len) {
2441-
case 0:
2442-
escape = PHP_CSV_NO_ESCAPE;
2443-
break;
2444-
case 1:
2445-
escape = (unsigned char) esc[0];
2446-
break;
2447-
default:
2448-
zend_argument_value_error(3, "must be empty or a single character");
2449-
RETURN_THROWS();
2450-
}
2451-
ZEND_FALLTHROUGH;
2452-
case 2:
2453-
if (e_len != 1) {
2454-
zend_argument_value_error(2, "must be a single character");
2455-
RETURN_THROWS();
2456-
}
2457-
enclosure = enclo[0];
2458-
ZEND_FALLTHROUGH;
2459-
case 1:
2460-
if (d_len != 1) {
2461-
zend_argument_value_error(1, "must be a single character");
2462-
RETURN_THROWS();
2463-
}
2464-
delimiter = delim[0];
2465-
ZEND_FALLTHROUGH;
2466-
case 0:
2467-
break;
2425+
if (zend_parse_parameters(ZEND_NUM_ARGS(), "|sss", &delim, &d_len, &enclo, &e_len, &esc, &esc_len) == FAILURE) {
2426+
RETURN_THROWS();
2427+
}
2428+
2429+
if (delim) {
2430+
if (d_len != 1) {
2431+
zend_argument_value_error(1, "must be a single character");
2432+
RETURN_THROWS();
24682433
}
2469-
intern->u.file.delimiter = delimiter;
2470-
intern->u.file.enclosure = enclosure;
2471-
intern->u.file.escape = escape;
2434+
delimiter = delim[0];
24722435
}
2436+
if (enclo) {
2437+
if (e_len != 1) {
2438+
zend_argument_value_error(2, "must be a single character");
2439+
RETURN_THROWS();
2440+
}
2441+
enclosure = enclo[0];
2442+
}
2443+
if (esc) {
2444+
if (esc_len > 1) {
2445+
zend_argument_value_error(3, "must be empty or a single character");
2446+
RETURN_THROWS();
2447+
}
2448+
if (esc_len == 0) {
2449+
escape = PHP_CSV_NO_ESCAPE;
2450+
} else {
2451+
escape = (unsigned char) esc[0];
2452+
}
2453+
}
2454+
2455+
intern->u.file.delimiter = delimiter;
2456+
intern->u.file.enclosure = enclosure;
2457+
intern->u.file.escape = escape;
24732458
}
24742459
/* }}} */
24752460

‎ext/spl/tests/SplFileObject_fputcsv_variation14.phpt

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,11 @@ try {
1515
} catch (ValueError $e) {
1616
echo $e->getMessage(), "\n";
1717
}
18+
try {
19+
var_dump($fo->fputcsv(array('water', 'fruit'), ',', '""'));
20+
} catch (ValueError $e) {
21+
echo $e->getMessage(), "\n";
22+
}
1823

1924
unset($fo);
2025

@@ -27,5 +32,6 @@ unlink($file);
2732
?>
2833
--EXPECT--
2934
*** Testing fputcsv() : with enclosure & delimiter of two chars and file opened in read mode ***
35+
SplFileObject::fputcsv(): Argument #2 ($separator) must be a single character
3036
SplFileObject::fputcsv(): Argument #3 ($enclosure) must be a single character
3137
Done

0 commit comments

Comments
 (0)